bugfix/Fix UnboundLocalError in TiffGlobReader

[ianhi]

Description

I passed a pd.Series of files to TiffGlobReader and got this error: UnboundLocalError: local variable 'file_series' referenced before assignment.

This prevents that error by raising a TypeError if passed an invalid type, and also allows for glob_in to be a pandas series.

As ever for tiffglobreader cc @jrussell25

Pull request recommendations:

• Name your pull request your-development-type/short-description. Ex: feature/read-tiff-files
• [NA] Link to any relevant issue in the PR description. Ex: Resolves [gh-], adds tiff file format support
• Provide relevant tests for your feature or bug fix.
• Provide or update documentation for any feature added by your pull request.

Thanks for contributing!

[evamaxfield]

Makes sense to me! Will get one more approval. Thanks!

[ianhi]

Does PathLike include pathlib.Path? I'm finding that if I pass a list of PosixPath objects I run into an error

[AetherUnbound]

Thanks for the contribution! Any chance you might be able to add some tests for this particular case as well? 😄

[evamaxfield]

Does PathLike include pathlib.Path? I'm finding that if I pass a list of PosixPath objects I run into an error

It should: https://github.com/AllenCellModeling/aicsimageio/blob/main/aicsimageio/types.py#L14

but without seeing the error I am not sure where it might be going wrong. Haven't looked at this code since the PR that added it

[ianhi]

edit: hang on I may have pasted the wrong thing

[codecov-commenter]

Codecov Report

Base: 93.90% // Head: 94.00% // Increases project coverage by +0.10% 🎉

Coverage data is based on head (0368167) compared to base (f789666).
Patch coverage: 95.83% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #449      +/-   ##
==========================================
+ Coverage   93.90%   94.00%   +0.10%     
==========================================
  Files          46       46              
  Lines        4001     4021      +20     
==========================================
+ Hits         3757     3780      +23     
+ Misses        244      241       -3     

Impacted Files	Coverage Δ
aicsimageio/readers/tiff_glob_reader.py	86.77% <85.71%> (+1.42%)	⬆️
aicsimageio/tests/readers/test_glob_reader.py	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[ianhi]

but without seeing the error I am not sure where it might be going wrong. Haven't looked at this code since the PR that added it

Somehow I've managed to end up with a nan in this line

aicsimageio/aicsimageio/readers/tiff_glob_reader.py

Line 338 in f789666

with TiffSequence(val.filename.tolist()) as tif:

In particular val.filename.tolist() is ['data/run_10/t012_p000_c001_z000.tiff', nan, 'data/run_10/t006_p006_c001_z001.tiff', 'data/run_10/t001_p003_c000_z002.tiff', 'data/run_10/t010_p006_c002_z002.tiff']

[ianhi]

Maybe related: I think that this line:

aicsimageio/aicsimageio/readers/tiff_glob_reader.py

Lines 176 to 178 in f789666

	elif isinstance(indexer, pd.DataFrame):
	self._all_files = indexer
	self._all_files["filename"] = file_series

should really be making a copy of the dataframe, as it stands the code modifies the object the user passes which can be confusing when you look back at the dataframe.

[ianhi]

Ok! I figured out the source of my nans!

folder = "data/run_10/"
files = pd.Series(Path(folder).glob("*.tiff"))
index = files.apply(indexer)

# remove time points where we didn't collect every position
idx = index_complete_timepoints(index)

# This line is causing the problem!
# makes the index.index no longer continuous
# this would all work, except that because I have to stringify filenames
# we lose the same treatment to the files index
# and then there are alignment issues 
index = index.loc[idx]
files = files.loc[idx]

# make list to work around https://github.com/AllenCellModeling/aicsimageio/pull/449
filenames = [str(f) for f in files]
reader = TiffGlobReader(filenames, indexer=index)

I need to filter out some files (because I had some partially completed timepoints) so the pandas indices were ending up misaligned because the filenames pd.Series has a continuous index

[ianhi]

Ok. Now has tests and I fixed the not copying dataframe/series, the index alignment, and also allowed np.arrays for the list of filenames

[evamaxfield]

Still good with this! Thanks for finding all the various oddities!

[AetherUnbound]

Looks great, thank you for adding tests! I have one suggested improvement to the test but nothing to stop a merge 😄

[ianhi]

ooh hang on. before merging I think I also need to update the typing on the parameters to reflect the new options.

Is it ok to go to Union[Pathlike, Iterable[Pathlike]] for glob_in?

[ianhi]

ooh hang on. before merging I think I also need to update the typing on the parameters to reflect the new options.

I think I did this successfully in the following two commits - so all good from me to merge :)

[evamaxfield]

Merging now! Thanks for the multiple bug fixes!