search

catalystneuro / nwb-conversion-tools

Create NWB files by converting and combining neural data in proprietary formats and adding essential metadata.
https://nwb-conversion-tools.readthedocs.io/en/main/
BSD 3-Clause "New" or "Revised" License
25 stars 12 forks source link

Add test for `ScanImageImagingInterface` in `test_gin_ophys.py`. #568

Closed h-mayorquin closed 2 years ago

h-mayorquin commented 2 years ago

We have some data on:

OPHYS_DATA_PATH / "imaging_datasets" / "Tif" / "sample_scanimage.tiff"

Should we add a conversion to nwb test with this data?

CodyCBakerPhD commented 2 years ago

Yes, when we add the ScanImageTiffDataInterface

h-mayorquin commented 2 years ago

@CodyCBakerPhD So this is different from this? https://github.com/catalystneuro/nwb-conversion-tools/blob/main/src/nwb_conversion_tools/datainterfaces/ophys/scanimage/scanimageimaginginterface.py

CodyCBakerPhD commented 2 years ago

Oops, forgot we already added that (I was looking for it under the 'Tiff' heading)

CodyCBakerPhD commented 2 years ago

Yeah, could you add that test in, then? Not sure how that slipped through the cracks

h-mayorquin commented 2 years ago

Oops, forgot we already added that (I was looking for it under the 'Tiff' heading)

Maybe it should be there?

CodyCBakerPhD commented 2 years ago

Maybe it should be there?

Depends on how we want things organized for discoverability - based only on their file suffixes, or on the software responsible for generating the file? It is a very slightly different format than normal .tiff files, so not exactly the same as the other interface

h-mayorquin commented 2 years ago

Maybe it should be there?

Depends on how we want things organized for discoverability - based only on their file suffixes, or on the software responsible for generating the file? It is a very slightly different format than normal .tiff files, so not exactly the same as the other interface

If you asked me before I would say the software, but the fact that you failed to found it is evidence for the other position.

CodyCBakerPhD commented 2 years ago

If you asked me before I would say the software, but the fact that you failed to found it is evidence for the other position.

lol 😂 I also might not have been trying that hard TBH, just a quick glance. Also source of the mental mistake might be that I originally (and arbitrarily, I thought, at the time) placed it according to suffix in ROIExtractors. So maybe that should be fixed to adhere to your preference of indexing by software?

h-mayorquin commented 2 years ago

I am really not sure, like I would not put cell explorer with something else that is .mat for example as it is highly specific. In this case though, I am thinking that they are similar enough that they should be together (I am thinking that we might be able to combine them in such a way that we cover all the cases and use the other as a fallback).

h-mayorquin commented 2 years ago

Closed as #581 is merged.