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

Metamorph: Correct DeltaT across channels #2922

Merged
merged 1 commit into from Aug 23, 2017

Conversation

dgault
Copy link
Member

@dgault dgault commented Aug 8, 2017

Issue was originally raised in forum thread https://www.openmicroscopy.org/community/viewtopic.php?f=13&t=8325
A sample set of files was also provided in QA-17824

In short each plane should have a unique delta T value, however each channel has the same timepoints as the that of the first channel. This is due to the fact that for the given sample file set, the channels are split across files but when determining which file to read in order to populate metadata the channel index is not taken into account.

Opening this PR to test against other filesets in the data repo

@rleigh-codelibre
Copy link
Contributor

This certainly looks fine, testing shows unique and reasonable deltaT values. Looks good to merge when the full repo test is green.

@sbesson
Copy link
Member

sbesson commented Aug 10, 2017

As shown by https://ci.openmicroscopy.org/job/BIOFORMATS-DEV-merge-repository-subset/463/ this will probably require a configuration change for one of our filesets. @dgault I propose to exclude this PR for now to get the repository jobs back to green and come back to this in a patch release assume it does not break serialization.

@dgault
Copy link
Member Author

dgault commented Aug 18, 2017

Associated QA PR opened: https://github.com/openmicroscopy/data_repo_config/pull/231

Removing from breaking.

@sbesson sbesson modified the milestone: 5.6.1 Aug 20, 2017
@sbesson
Copy link
Member

sbesson commented Aug 21, 2017

Tested with the QA provided with the initial forum post. Everything worked as expected and the planetimestamps in the second channel are properly offset from the plane timestamps in the first channel.

The configuration file changes for our internal QA file also shows the same offset behavior. In addition, the values of the Zposition values are also slightly offset between the various channels.

Discussed the latter point with @dgault, while the Plane dimension indices is conserved to express the mapping between planes, this change also provides the correct representation for the plane acquisition metadata (position , timestamp).

Also checked these changes do not break serialization from 5.6.0. Once validated by the full repository tests, this PR should be good to merge.

@sbesson sbesson merged commit 672b27e into ome:develop Aug 23, 2017
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.

None yet

3 participants