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

Update BrukerTiffImagingExtractor #230

Merged
merged 25 commits into from
Aug 7, 2023

Conversation

weiglszonja
Copy link
Contributor

Update BrukerTiffImagingExtractor to handle the additional types of Bruker data (volumetric, multi color, stimulation)
WIP

@weiglszonja weiglszonja marked this pull request as draft June 30, 2023 12:21
@weiglszonja weiglszonja self-assigned this Jun 30, 2023
@CodyCBakerPhD
Copy link
Member

Thanks @weiglszonja

Going to enlist the help of @pauladkisson to help review this PR to get them some more experience with the workflow of managing this repository

Copy link
Member

@pauladkisson pauladkisson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look pretty reasonable to me. It looks like CI checks are failing due to file/path issues, could you clear that up? Or if you're using a file that isn't in the ophys testing repo, we should probably add it.

@weiglszonja
Copy link
Contributor Author

weiglszonja commented Jul 6, 2023

Thanks @pauladkisson, the PR for uploading the test data is here, once it's merged hopefully the tests will pass.
I'd still prefer to keep this in draft until I receive the multichannel example. (so that I can try and see if there is anything else to modify)

@weiglszonja
Copy link
Contributor Author

weiglszonja commented Jul 10, 2023

Notes for Bruker version (can be checked in the XML file)

  • files that were created with version 5.6.64 are individual .ome.tif files (dual plane example, single color example)
  • files that were created with version 5.8.64 are multipage .ome.tif files (dual color example here)
    The number of files for the multipage tiffs can depend on the number of channels, but I also think there can be more files per channel depending on the chunk size that was used (e.g. ~500MB). Also possible that even with the new version the user can choose to create individual tiffs, but that is being confirmed here.

Adding @h-mayorquin to this thread as he is also working with dual channel data.

@CodyCBakerPhD
Copy link
Member

Unfortunately it looks like @h-mayorquin's data, though it was originally Bruker output, was converted into another format (nifty) after acquisition (and source was not kept) so the example data we have here is from Pinto lab and the NWB helpdesk (best volumetric example so far)

weiglszonja and others added 8 commits July 18, 2023 17:43
CodyCBakerPhD
CodyCBakerPhD previously approved these changes Aug 7, 2023
@CodyCBakerPhD
Copy link
Member

LGTM! Thanks for all the hard work on this

@codecov
Copy link

codecov bot commented Aug 7, 2023

Codecov Report

Merging #230 (31bb80a) into main (b7c6048) will increase coverage by 0.98%.
The diff coverage is 96.29%.

❗ Current head 31bb80a differs from pull request most recent head 16a606c. Consider uploading reports for the commit 16a606c to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #230      +/-   ##
==========================================
+ Coverage   76.15%   77.13%   +0.98%     
==========================================
  Files          37       37              
  Lines        2583     2725     +142     
==========================================
+ Hits         1967     2102     +135     
- Misses        616      623       +7     
Flag Coverage Δ
unittests 77.13% <96.29%> (+0.98%) ⬆️

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

Files Changed Coverage Δ
...iffimagingextractors/brukertiffimagingextractor.py 97.00% <96.29%> (-0.82%) ⬇️

... and 1 file with indirect coverage changes

@CodyCBakerPhD CodyCBakerPhD marked this pull request as ready for review August 7, 2023 15:41
@CodyCBakerPhD CodyCBakerPhD merged commit 875bd0e into main Aug 7, 2023
12 checks passed
@CodyCBakerPhD CodyCBakerPhD deleted the update_BrukerTiffImagingExtractor branch August 7, 2023 15:49
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