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

Andor - Plane Position Z #2117

Merged
merged 3 commits into from Dec 4, 2015
Merged

Andor - Plane Position Z #2117

merged 3 commits into from Dec 4, 2015

Conversation

dgault
Copy link
Member

@dgault dgault commented Nov 25, 2015

Reported Issue: http://lists.openmicroscopy.org.uk/pipermail/ome-users/2015-November/005745.html

This is a follow up to : #1997

To reproduce:

  • Using uploaded from file QA 16928
  • Import with bioformats and view metadata
  • Verify that the PhysicalSizeZ and TheZ are set correctly and PositionZ is 0 for each image

To test the PR:

  • Follow the same steps above but verify that PositionZ is now set for each image, PositionZUnit should also now be µm rather than reference frame

@dgault
Adding Plane Position Z values
6becdd2 
Using the zPositions read from image metadata to populate
PlanePositionZ for each image
@sbesson
Copy link
Member

sbesson commented Nov 30, 2015

Tested by generating the OME-XML with and without this PR on the sample file

$ showinf -nopix -omexml-only 16928/2Ch_Z10_T10.tif > /tmp/2Ch_Z10_T10.ome
$ /opt/ome/bioformats/tools/showinf -omexml-only 16928/2Ch_Z10_T10.tif  > /tmp/2Ch_Z10_T10_2117.ome

and computing the diff between headers. As described in the PR, the only difference is the PositionZ which is now properly filled for all the planes. Do you have the bandwidth to add a matching configuration file to test the QA file and prevent future regressions?

@dgault
Copy link
Member Author

dgault commented Nov 30, 2015

I have generated a matching config for the QA file, however it has thrown up a small bug in the test-suite for zPosition units. I have a fix for the test suite bug, but should it be opened in a separate PR?

@sbesson
Copy link
Member

sbesson commented Nov 30, 2015

If the test-suite changes have been tested and fixed in the context of this PR, feel free to add the commit here.

@dgault
Copy link
Member Author

dgault commented Nov 30, 2015

Matching config PR for new sample file can be found - https://github.com/openmicroscopy/data_repo_config/pull/51

@dgault
Generate correct units for YPos and ZPos
fb32e8f 
Configuration for Position Y and Position Z had been using the same
units as Position X. Modifying to allow for correct units to be used
for each dimension
@dgault
Copy link
Member Author

dgault commented Nov 30, 2015

Added a final commit to this PR to fix the units for Y and Z Positions in test-suite configuration

@dgault
Copy link
Member Author

dgault commented Dec 1, 2015

Updated the test suite further as reference frame was hardcoded as the expected unit type for plane positions.

https://ci.openmicroscopy.org/job/BIOFORMATS-5.1-merge-repository-subset/191/

@melissalinkert melissalinkert mentioned this pull request Dec 1, 2015
Closed
@sbesson
Copy link
Member

sbesson commented Dec 4, 2015

See https://ci.openmicroscopy.org/view/Failing/job/BIOFORMATS-5.1-merge-full-repository/696/

sbesson added a commit that referenced this pull request Dec 4, 2015
@sbesson
Merge pull request #2117 from dgault/Andor-ZPos
c353c54 
Andor - Plane Position Z
@sbesson sbesson merged commit c353c54 into ome:dev_5_1 Dec 4, 2015
@sbesson sbesson added this to the 5.1.7 milestone Dec 4, 2015
@dgault dgault mentioned this pull request Dec 4, 2015
Merged
@dgault
Copy link
Member Author

dgault commented Dec 4, 2015

--rebased-to #2136

@dgault dgault mentioned this pull request Dec 4, 2015
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
dev_5_1
Projects
None yet
Milestone
5.1.7
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@dgault @sbesson