-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
draft of multi-tiff multi-page imaging extractor #323
base: main
Are you sure you want to change the base?
Conversation
…e used on its own or used as a base type for more specialized extractors
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
…actor' into multitiff_multipage_imaging_extractor
for more information, see https://pre-commit.ci
…actor' into multitiff_multipage_imaging_extractor # Conflicts: # tests/test_multitiff_multipage_imaging_extractor.py
for more information, see https://pre-commit.ci
…actor' into multitiff_multipage_imaging_extractor
I see it might be been simpler to use a MultiImagingExtractor. I'll look at that now |
for more information, see https://pre-commit.ci
…actor' into multitiff_multipage_imaging_extractor # Conflicts: # requirements-full.txt
for more information, see https://pre-commit.ci
…actor' into multitiff_multipage_imaging_extractor
for more information, see https://pre-commit.ci
…actor' into multitiff_multipage_imaging_extractor
@CodyCBakerPhD I'm not sure what's going on here. This code runs perfectly well when I run it locally. |
Oh, right, because the test data hasn't been merged yet, so there are no extractors. I'll add a check for that. |
Here's what I propose for this approach: i) A class called ii) A class called I think each could have its own interface on NeuroConv, but GUIDE would only support (i) While (ii) is technically possible in ROIExtractors out of the box by just making a list of TiffImagingExtractors and putting them in a MultiImagingExtractor, having a dedicated extractor could still be more convenient or findable @bendichter WDYT? |
"split_1.tif", | ||
"split_2.tif", | ||
"split_3.tif", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some extra tests for this, at a minimum, should include cases of (a) multiple cameras/microscopes (the specifier for each device being something in the file name), and (b) multiple optic channels for a single one of those devices (the channel name/ID also being a part of the filename)
Not that I'm not saying the TiffFolderImagingExtractor
should by default support multiple optic channels, or auto-detection of channel IDs from this pattern (that is a more complicated feature that could be done in a follow-up, or not at all); I just want to make sure that a single instance made from a pattern specification that restricts to only one channel from a folder that contains several correctly identifies list of files only for that channel
Yeah I think these are both good ideas |
@CodyCBakerPhD the thing is I also want to create an extractor that handles a bunch of TIFF files that are not multi-page. What do you think of this naming scheme?
Then you could have |
I think that would be the mosts straightforward syntax for the user. The issue is I don't really want to inherit from MultiImagingInterface for the "many 1-page TIFF files" case. |
…actor' into multitiff_multipage_imaging_extractor
Sounds good to me Quick question - can the detail about a file being multi-page be autodetected or is it really something that needs to be known prior to reading the file? |
@CodyCBakerPhD I suppose you are right, we could auto-detect multi-page. I'll put together a draft of that. We might need to use |
for more information, see https://pre-commit.ci
This could be used on its own or used as a base type for more specialized extractors.
Depends on https://gin.g-node.org/CatalystNeuro/ophys_testing_data/pulls/21