Conversation
hamshkhawar
requested review from
NHotaling and
Nicholas-Schaub
as code owners
nishaq503
left a comment
nishaq503
left a comment
There was a problem hiding this comment.
This look to be in pretty good shape. I have left comments for things that need to be updated.
There was a problem hiding this comment.
This needs to be formatted with proper markdown syntax. See here for a proper CHANGELOG format:
| ".ome.tif", | ||
| ) | ||
| out_json["outDir"].append(out_name) | ||
| json.dump(out_json, jfile, indent=2) |
There was a problem hiding this comment.
When running with the preview option, we should return here and not run the rest of the code.
|
|
||
| data = s.data_segment().data().reshape(dims) | ||
|
|
||
| write_thread(out_file_path, data, metadata, chan_names[c]) |
There was a problem hiding this comment.
You've done all the ground work for making this multithreaded. Please actually use preadator here.
There was a problem hiding this comment.
This file should be named conftest.py. Then, it will not need to be explicitly imported in the tests because pytest will handle it.
| def clean_directories() -> None: | ||
| """Remove all temporary directories.""" | ||
| for d in Path(".").cwd().iterdir(): | ||
| if d.is_dir() and d.name.startswith("tmp"): | ||
| shutil.rmtree(d) |
There was a problem hiding this comment.
I am downloading large czi files and dont want to download it again for every test. which is why I am cleaning directories after running tests
| @pytest.fixture() | ||
| def output_directory() -> Union[str, Path]: | ||
| """Create output directory.""" | ||
| return Path(tempfile.mkdtemp(dir=Path.cwd())) |
There was a problem hiding this comment.
Do not pass the dir parameter here. tempfile will automatically determine the correct place for the directory. Further, when the tests end (regardless of whether they succeeded or failed), the temp directories will automatically be cleaned up.
| row_conv = dict( | ||
| zip( | ||
| np.unique(np.sort(ind["Y"])), | ||
| range(0, len(np.unique(ind["Y"]))), | ||
| ), | ||
| ) | ||
| col_conv = dict( | ||
| zip( | ||
| np.unique(np.sort(ind["X"])), | ||
| range(0, len(np.unique(ind["X"]))), | ||
| ), | ||
| ) |
There was a problem hiding this comment.
Store the unique values in a variable, then sort them. Otherwise, this would be very slow for large archives.
| row=ind["Row"][i], | ||
| col=ind["Col"][i], |
There was a problem hiding this comment.
Do all CZI archives contain FOVs that have row and column information? If not, then this code could break.
There was a problem hiding this comment.
Now it is handled in a code if function is unable to extract unique fovs in czi file.
hamshkhawar
force-pushed
the
update_czi
branch
from
f0f3c69 to
594e140
Compare
3 participants
This plugin is updated to latest plugin standards
Updated dependencies and included filepattern for file parsing
Wrote pytests for this plugin