Skip to content
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

add BrukerTiffImagingExtractor #220

Merged
merged 27 commits into from
Apr 24, 2023
Merged

Conversation

weiglszonja
Copy link
Contributor

Add support for Bruker 2P imaging data which consists of:

  • individual TIFF files in OME-TIF format (.ome.tif files) where each TIFF file corresponds to a single frame
  • metadata in XML format (.xml and .env files)

The metadata from the .xml file in dict form for reference:

{
'version': '5.6.64.400',
'date': '2/20/2023 3:58:25 PM',
'notes': '',
'activeMode': 'ResonantGalvo',
'bitDepth': '13',
'currentScanAmplitude': [{'XAxis': '4.17170481'}, {'YAxis': '4.17170481'}],
'currentScanCenter': [{'XAxis': '0'}, {'YAxis': '0'}],
'dwellTime': '0.4',
'framePeriod': '0.032953338',
'interlacedScanTrackCount': '0',
'laserPower': [{'920 Axon': '500'}, {'1064 Axon': '900.390625'}, {'Monaco': '0'}],
'linesPerFrame': '512',
'maxVoltage': [{'XAxis': '2.085852405'}, {'YAxis': '2.085852405'}],
'micronsPerPixel': [{'XAxis': '1.1078125'}, {'YAxis': '1.1078125'}, {'ZAxis': '5'}],
'minVoltage': [{'XAxis': '-2.085852405'}, {'YAxis': '-2.085852405'}],
'objectiveLens': '16X', 'objectiveLensMag': '16', 'objectiveLensNA': '0.8',
'opticalZoom': '2',
'pixelsPerLine': '512',
'pmtGain': [{'PMT 1 HV (Red)': '800'}, {'PMT 2 HV (Green)': '700'}],
'positionCurrent': [{'XAxis': '0'}, {'YAxis': '0'}, {'Z Focus': '147.55'},
                     {'Optotune ETL 10-30': '-80.0653860309135'}],
 'preampFilter': 'NoFilter',
 'preampOffset': [{'Ch1': '0'}, {'Ch2': '0'}],
 'rastersPerFrame': '1',
 'rotation': '0',
 'samplesPerPixel': '3',
 'scanLinePeriod': '6.3129E-05',
 'useInterlacedScanPattern': 'False',
 'xYStageGridIndex': '-1', 'xYStageGridXIndex': '-1', 'xYStageGridYIndex': '-1',
 'yAspectExpansion': '1', 'zDevice': '0'}

@weiglszonja weiglszonja self-assigned this Mar 31, 2023
@weiglszonja weiglszonja marked this pull request as draft March 31, 2023 16:02
@weiglszonja
Copy link
Contributor Author

weiglszonja commented Mar 31, 2023

TODO: add tests (haven't found test data to upload to test folder)
TODO: Add a DynamicTable with the metadata shown in the description

@CodyCBakerPhD
Copy link
Member

TODO: Bruker metadata extension? with the metadata shown in the description

How long would that take compared to writing a DynamicTable with a single row, whose columns are each of the fields? (both approaches include descriptions of each field, but the data type would be auto-inferred in the table case)

@CodyCBakerPhD
Copy link
Member

TODO: add tests (haven't found test data to upload to test folder)

How big is this session of data? Would including only a handful of the first tiffs (basically a stub) cause a problem for the reader? (like a mismatch with something expected from the header)

@weiglszonja
Copy link
Contributor Author

weiglszonja commented Mar 31, 2023

TODO: Bruker metadata extension? with the metadata shown in the description

How long would that take compared to writing a DynamicTable with a single row, whose columns are each of the fields? (both approaches include descriptions of each field, but the data type would be auto-inferred in the table case)

You're right, I haven't considered that approach, makes sense!

TODO: add tests (haven't found test data to upload to test folder)

How big is this session of data? Would including only a handful of the first tiffs (basically a stub) cause a problem for the reader? (like a mismatch with something expected from the header)

~11.81GB on disk total. Currently it would, but it can be changed that I ignore what is in the header and assume the number of frames from the tif files within the folder (but also make sure they are in consecutive order, which is the case when collecting them from the header)

@CodyCBakerPhD
Copy link
Member

Currently it would, but it can be changed that I ignore what is in the header and assume the number of frames from the tif files within the folder (but also make sure they are in consecutive order, which is the case when collecting them from the header)

What field of the header describes how many frames to expect?

@bendichter
Copy link
Contributor

I think it's better to build an extension for Bruker rather than using a DynamicTable

@weiglszonja
Copy link
Contributor Author

weiglszonja commented Mar 31, 2023

Currently it would, but it can be changed that I ignore what is in the header and assume the number of frames from the tif files within the folder (but also make sure they are in consecutive order, which is the case when collecting them from the header)

What field of the header describes how many frames to expect?

It is not a single field, each frame has an entry like this:

    </Frame>
    <Frame relativeTime="602.234763984" absoluteTime="619.099763983991" index="17992" parameterSet="CurrentSettings">
      <File channel="2" channelName="Ch2" filename="NCCR32_2023_02_20_Into_the_void_t_series_baseline-000_Cycle00001_Ch2_017992.ome.tif" />
      <ExtraParameters lastGoodFrame="0" />
      <PVStateShard>
        <PVStateValue key="framePeriod" value="0.033474224" />
        <PVStateValue key="scanLinePeriod" value="6.3136E-05" />
      </PVStateShard>
    </Frame>
    <Frame relativeTime="602.268238208" absoluteTime="619.133238207991" index="17993" parameterSet="CurrentSettings">
      <File channel="2" channelName="Ch2" filename="NCCR32_2023_02_20_Into_the_void_t_series_baseline-000_Cycle00001_Ch2_017993.ome.tif" />
      <ExtraParameters lastGoodFrame="0" />
      <PVStateShard>
        <PVStateValue key="framePeriod" value="0.033474224" />
        <PVStateValue key="scanLinePeriod" value="6.3136E-05" />
      </PVStateShard>
    </Frame>
    <Frame relativeTime="602.301712432" absoluteTime="619.166712431991" index="17994" parameterSet="CurrentSettings">
      <File channel="2" channelName="Ch2" filename="NCCR32_2023_02_20_Into_the_void_t_series_baseline-000_Cycle00001_Ch2_017994.ome.tif" />
      <ExtraParameters lastGoodFrame="0" />
      <PVStateShard>
        <PVStateValue key="framePeriod" value="0.033474224" />
        <PVStateValue key="scanLinePeriod" value="6.3136E-05" />
      </PVStateShard>
    </Frame>

@CodyCBakerPhD
Copy link
Member

It is not a single field, each frame has an entry like this:

Interesting...

But right now you decide the iteration from

    def _get_files_names(self):
        return [file.attrib["filename"] for file in self._get_xml_root().findall(".//File")]

which seems similar to globbing...

From there, if you wanted to confirm that the index of the last Frame matched the number of files from globbing, it would only be to ensure the two are consistent, right?

So to create a stub, we would remove all the frame subfields whose indexes correspond to the files we would remove as well? Since there isn't a global counter that seems a little easier

weiglszonja and others added 3 commits April 3, 2023 13:04
…magingextractor.py

Co-authored-by: Cody Baker <51133164+CodyCBakerPhD@users.noreply.github.com>
@CodyCBakerPhD
Copy link
Member

Last 2 items then are

  1. docstrings

and

  1. tests (requires a stubbed session)

@CodyCBakerPhD
Copy link
Member

@weiglszonja OK CI now has the data on its cache

Just running into some actual errors now

@weiglszonja
Copy link
Contributor Author

@CodyCBakerPhD Thanks, I see it now. I think the issue is that (but not sure how to check in the CI which tifffile version is installed), but in the requirements_full.txt it uses an older version:

tifffile==2018.10.18

and I was using 2023.3.21.
I checked in neuroconv and TiffImagingInterface uses tiffile>=2018.10.18.
Can we change it here too?

@CodyCBakerPhD
Copy link
Member

Can we change it here too?

Do w/e you need to do to get the CI passing here, yeah

@weiglszonja weiglszonja marked this pull request as ready for review April 20, 2023 17:32
@codecov
Copy link

codecov bot commented Apr 21, 2023

Codecov Report

Merging #220 (4f1b3f5) into master (247ff5d) will increase coverage by 0.99%.
The diff coverage is 97.87%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #220      +/-   ##
==========================================
+ Coverage   73.28%   74.27%   +0.99%     
==========================================
  Files          33       34       +1     
  Lines        2302     2395      +93     
==========================================
+ Hits         1687     1779      +92     
- Misses        615      616       +1     
Flag Coverage Δ
unittests 74.27% <97.87%> (+0.99%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...iffimagingextractors/brukertiffimagingextractor.py 97.82% <97.82%> (ø)
src/roiextractors/extractorlist.py 100.00% <100.00%> (ø)
...ctors/extractors/tiffimagingextractors/__init__.py 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

@CodyCBakerPhD CodyCBakerPhD merged commit 4494155 into master Apr 24, 2023
@CodyCBakerPhD CodyCBakerPhD deleted the add_BrukerTiffImagingExtractor branch April 24, 2023 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants