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

Generalized wavelength extraction & fix xy mixup in ROI tables #36

Merged
merged 3 commits into from
Nov 8, 2023

Conversation

jluethi
Copy link
Contributor

@jluethi jluethi commented Jul 11, 2023

This addresses #35

I based it on the #32 branch, as I'm working from that. If necessary, I can also rebase to the current main. But I'd suggest we merge this after #32.
=> only the last commit, 02652b9, is new.

This reads out any metadata entry that ends in "Intensity", which just appear to be the ones we're interested in and works both for FMI setups & ZMB setups. The metadata doesn't contain a list of wavelengths or a similar entry we could use to get the definitive list of channels.

For backwards compatibility, I included an optional renaming of the wavelengths to the strings used before. For the more general case, I'd suggest we go with whatever string the microscope metadata provides.

@jluethi
Copy link
Contributor Author

jluethi commented Jul 14, 2023

We just realized that our generalized handling of the Intensity metadata was now working insofar as processing wouldn't crash when different names are given to the Intensity metadata. But on the ZMB side, the data was still not correctly added to the OME-Zarr metadata yet. To achieve that, we also had to generalize which Intensity metadata is read in in the MetaSeriesTiff part. See latest commit for that

@codecov
Copy link

codecov bot commented Jul 19, 2023

Codecov Report

Merging #36 (1461e91) into main (e9fd767) will increase coverage by 2.29%.
The diff coverage is 95.00%.

@@            Coverage Diff             @@
##             main      #36      +/-   ##
==========================================
+ Coverage   93.76%   96.05%   +2.29%     
==========================================
  Files          16       16              
  Lines        1315     1293      -22     
==========================================
+ Hits         1233     1242       +9     
+ Misses         82       51      -31     
Files Coverage Δ
src/faim_hcs/MontageUtils.py 97.75% <ø> (ø)
src/faim_hcs/io/MetaSeriesTiff.py 100.00% <100.00%> (ø)
tests/test_MetaSeriesUtils.py 100.00% <100.00%> (ø)
tests/test_Zarr.py 99.26% <ø> (ø)
src/faim_hcs/MetaSeriesUtils.py 96.93% <93.33%> (+16.08%) ⬆️

@jluethi jluethi changed the title Generalized wavelength extraction Generalized wavelength extraction & fix xy mixup in ROI tables Sep 15, 2023
@jluethi
Copy link
Contributor Author

jluethi commented Sep 15, 2023

Quick overview of this PR:
This is our current working branch of faim-hcs we're using for Fractal tasks. It got much simpler now that #32 is merged. Thanks!
We're proposing 2 small changes to the metadata parsing here. Plus a fix for an issue with the ROI tables.

Both metadata changes are to make sure the parser works for MDs with differently named intensity channels (like the one at ZMB).

Metadata change 1 in MetaSeriesUtils:
The get_wavelength_power was specific to the names like "Lumencor Cyan Intensity". We added a more general approach by processing all metadata entries that end in "Intensity".
Because specific names were assigned for the FMI channels, we added a custom_channel_dict and rename the FMI channels to those names, as the function did before.

Metadata change 2 in MetaSeriesTiff:
The names of the intensity channels were hard-coded. As above, they aren't always available under these names. Instead, we're suggesting to just read in all the metadata fields that end in "Intensity" instead.

Change to ROI tables:
We realized during more thorough testing today that the x & y columns in the ROI tables were swapped. This PR is proposing to correct that, actually assigning x values to the x column.

This PR is work together with @fstur

@imagejan imagejan force-pushed the generalized_wavelength_extraction branch from 14355d1 to 1461e91 Compare November 8, 2023 10:33
@imagejan
Copy link
Member

imagejan commented Nov 8, 2023

I rebased onto current main and resolved the conflicts (some functions moved into MontageUtils.py). Will merge as soon as all checks pass.

@imagejan imagejan merged commit f8c6165 into fmi-faim:main Nov 8, 2023
6 checks passed
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.

2 participants