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.

Sign up for GitHub

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

Jump to bottom

Include Omero-channels-metadata update in import-ome-zarr task #579

Merged
merged 30 commits into from
Oct 20, 2023
Merged

Include Omero-channels-metadata update in import-ome-zarr task #579

merged 30 commits into from
Oct 20, 2023

Conversation

tcompa
Copy link
Collaborator

@tcompa tcompa commented Oct 19, 2023 •
edited
Loading

Checklist before merging

  • I added an appropriate entry to CHANGELOG.md

@tcompa tcompa linked an issue Oct 19, 2023 that may be closed by this pull request
Include Omero-channels-metadata update in import-ome-zarr task #578
Closed
@github-actions
Copy link

github-actions bot commented Oct 19, 2023 •
edited
Loading

Coverage report

The coverage rate went from 90.37% to 90.5% ⬆️
The branch rate is 84%.

None of the new lines are part of the tested code. Therefore, there is no coverage data about them.

@tcompa tcompa mentioned this pull request Oct 19, 2023
Closed
@tcompa
Copy link
Collaborator Author

tcompa commented Oct 19, 2023 •
edited
Loading

Last missing point from #578:

  • Assess whether we also need to add values for Window and/or colormap, if missing;

@tcompa tcompa marked this pull request as ready for review October 19, 2023 13:30
@tcompa
Copy link
Collaborator Author

tcompa commented Oct 19, 2023

This is ready for a review, and it will be the last contribution to the upcoming v0.13.0.

@tcompa tcompa requested a review from jluethi October 19, 2023 13:31
@tcompa
Copy link
Collaborator Author

tcompa commented Oct 19, 2023

(see here for a recap: #578 (comment))

jluethi
jluethi approved these changes Oct 19, 2023
View reviewed changes
Copy link
Collaborator

@jluethi jluethi left a comment

Choose a reason for hiding this comment

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

Looks great! Besides the 2 minor remarks on a comment & a message in the NotImplementedError, looks good to merge from my side!

fractal_tasks_core/lib_channels.py Show resolved Hide resolved
fractal_tasks_core/lib_channels.py Outdated Show resolved Hide resolved
fractal_tasks_core/tasks/import_ome_zarr.py Outdated Show resolved Hide resolved
@tcompa
Copy link
Collaborator Author

tcompa commented Oct 20, 2023

Thanks for the review, merging now

@tcompa tcompa merged commit 1df67ca into main Oct 20, 2023
17 checks passed
@tcompa tcompa deleted the 578-should-import-ome-zarr-add-wavelength_id-andor-label-information-into-omero-metadata branch October 20, 2023 06:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jluethi jluethi jluethi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Include Omero-channels-metadata update in import-ome-zarr task
2 participants
@tcompa @jluethi