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

Fix the restoration of non-indexed axes. #1189

Merged
merged 1 commit into from
Apr 18, 2019
Merged

Fix the restoration of non-indexed axes. #1189

merged 1 commit into from
Apr 18, 2019

Conversation

ttung
Copy link
Collaborator

@ttung ttung commented Apr 16, 2019

When the selection removes a dimension, xarray.expand_dims does not expand the non-indexed dimensions that were removed. For example, if one selects only a single zplane, it reduce the z physical coordinate to a coordinate scalar, and not an array of size 1.

When we detect that an axis was reduced out of existence, we ensure that the dependent axes are restored to arrays.

Test plan: Wrote a test that selects an ImageStack using a variety of selectors. In each case, the length of each indexed dimension (i.e., X, Y, ZPLANE) must match its dependent dimension (i.e., XC, YC, ZC).

@ttung ttung mentioned this pull request Apr 16, 2019
Merged
@codecov-io
Copy link

codecov-io commented Apr 16, 2019 •
edited
Loading

Codecov Report

Merging #1189 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1189      +/-   ##
==========================================
+ Coverage    88.5%   88.51%   +<.01%     
==========================================
  Files         133      133              
  Lines        4978     4982       +4     
==========================================
+ Hits         4406     4410       +4     
  Misses        572      572
Impacted Files Coverage Δ
starfish/imagestack/indexing_utils.py 95.34% <100%> (+0.47%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0ab4ff1...93dd7d2. Read the comment docs.

@ttung ttung force-pushed the tonytung-nonindexed-expansion branch from 19cdaa2 to 3972c7c Compare April 16, 2019 16:00
shanaxel42
shanaxel42 reviewed Apr 16, 2019
View reviewed changes
Copy link
Collaborator

@shanaxel42 shanaxel42 left a comment

Choose a reason for hiding this comment

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

I don't totally understand the issue? If you sel on value of the zplane then that results in the zc dimension only having a scalar value? But does that matter? You could still access that value with indexed.xarray['zc'].data

@ttung ttung force-pushed the tonytung-nonindexed-expansion branch from 3972c7c to 654fb29 Compare April 17, 2019 02:39
@ttung ttung force-pushed the tonytung-nonindexed-expansion branch from 654fb29 to c31e22f Compare April 17, 2019 03:38
@ttung ttung force-pushed the tonytung-nonindexed-expansion branch from c31e22f to 0ba45b6 Compare April 17, 2019 04:52
@ttung
Copy link
Collaborator Author

ttung commented Apr 17, 2019

I don't totally understand the issue? If you sel on value of the zplane then that results in the zc dimension only having a scalar value? But does that matter? You could still access that value with indexed.xarray['zc'].data

ImageStack.export indexes into xarray['zc'] to retrieve the coordinates of each tile. If it's a scalar, that operation does not work.

@ttung ttung force-pushed the tonytung-nonindexed-expansion branch 2 times, most recently from 5d8b588 to 71202db Compare April 17, 2019 07:26
@ttung ttung force-pushed the tonytung-nonindexed-expansion branch from 71202db to d42fa0f Compare April 17, 2019 17:29
@ttung ttung force-pushed the tonytung-nonindexed-expansion branch from d42fa0f to 9af3aec Compare April 18, 2019 00:11
Fix the restoration of non-indexed axes.
93dd7d2 
When the selection removes a dimension, xarray.expand_dims does not expand the non-indexed dimensions that were removed.  For example, if one selects only a single zplane, it reduce the z physical coordinate to a coordinate scalar, and not an array of size 1.

When we detect that an axis was reduced out of existence, we ensure that the dependent axes are restored to arrays.
@ttung ttung force-pushed the tonytung-nonindexed-expansion branch from 9af3aec to 93dd7d2 Compare April 18, 2019 05:22
@ttung ttung merged commit a4645e6 into master Apr 18, 2019
ttung pushed a commit that referenced this pull request Apr 18, 2019
Avoid use of _tile_data.keys()
561135c 
`_tile_data.keys()` returns tiles outside the scope of the imagestack if it has been subselected.  Instead, use `_iter_axes(..)`, which uses the actual xarray coordinates to determine which tiles exist.

Add a test that selects and exports an imagestack.

Depends on #1189
Fixes #1154
ttung added a commit that referenced this pull request Apr 18, 2019
@ttung
Avoid use of _tile_data.keys() (#1190)
380b65c 
`_tile_data.keys()` returns tiles outside the scope of the imagestack if it has been subselected.  Instead, use `_iter_axes(..)`, which uses the actual xarray coordinates to determine which tiles exist.

Add a test that selects and exports an imagestack.

Depends on #1189
Fixes #1154
@ttung ttung deleted the tonytung-nonindexed-expansion branch April 18, 2019 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shanaxel42 shanaxel42 shanaxel42 approved these changes

@ambrosejcarr ambrosejcarr Awaiting requested review from ambrosejcarr

@kne42 kne42 Awaiting requested review from kne42

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

Successfully merging this pull request may close these issues.
3 participants
@ttung @codecov-io @shanaxel42