Clean up binary mask #1622
Merged
Clean up binary mask #1622
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #1622 +/- ##
==========================================
- Coverage 90.09% 90.09% -0.01%
==========================================
Files 234 234
Lines 8764 8761 -3
==========================================
- Hits 7896 7893 -3
Misses 868 868
Continue to review full report at Codecov.
|
ttung
force-pushed
the
tonytung-binary-mask-cleanup
branch
2 times, most recently
from
e231f98 to
69253ca
Compare
ttung
force-pushed
the
tonytung-binary-mask-cleanup
branch
from
69253ca to
b0301db
Compare
ttung
force-pushed
the
tonytung-binary-mask-cleanup
branch
3 times, most recently
from
88e5b1a to
8512135
Compare
1. Use typed axes/coordinates when possible. 2. Have `_get_axes_names` return axes/coordinates in the order they are expected in the numpy arrays. 3. Improve how we render the mask names. Test plan: `pytest starfish/core/binary_mask/test`
ttung
force-pushed
the
tonytung-binary-mask-cleanup
branch
from
8512135 to
b8bfd88
Compare
shanaxel42
approved these changes
Nov 12, 2019
|
|
||
| Returns | ||
| ------- | ||
| masks : BinaryMaskCollection | ||
| Masks generated from the label image. | ||
| """ | ||
| props = regionprops(label_image) | ||
| len_max_label = len(str(len(props) - 1)) |
There was a problem hiding this comment.
why is there the extra str conversion here?
There was a problem hiding this comment.
I was tripped up by this too. Let's say we have 100 regions. len(props) - 1 gets you 99 as an int. Then str(99) gives you "99". Then len("99") gives you 2.
|
|
||
|
|
||
| def fill_from_mask( | ||
| mask: xr.DataArray, | ||
| fill_value: int, | ||
| result_array: np.ndarray, | ||
| axes_order: Sequence[Union[str, Axes]] = AXES_ORDER, | ||
| axes_order: Sequence[Union[str, Axes]] = (Axes.ZPLANE, Axes.Y, Axes.X), |
There was a problem hiding this comment.
why not use your get_axes_order helper method here?
There was a problem hiding this comment.
That basically happens in #1628
ttung
pushed a commit
that referenced
this pull request
Nov 13, 2019
LabelImage requires all the pixel coordinate and physical coordinate values along each axis, so to facilitate the easy conversion to and from BinaryMaskCollection and LabelImage, we need to change the data model for BinaryMaskCollection. Previously, BinaryMaskCollection stored a series of xarrays for each of the masks. This does not guarantee that the pixel/physical coordinates for every position along each axis. Furthermore, it duplicates a lot of data. This new approach stores the coordinates as independent arrays, and then stores each mask as a naked np.ndarray with offsets. Conversions to and from LabelImage is pretty straightforward. Returning each mask as an xarray (to maintain compatibility with the existing API) requires generating an xarray on the fly, but is not terribly expensive. Writing to disk is now through a versioned interface similar to what was done for slicedimage. Test plan: Updated tests to reflect the API changes. Part of #1497 Depends on #1619, #1622
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
_get_axes_namesreturn axes/coordinates in the order they are expected in the numpy arrays.Test plan:
pytest starfish/core/binary_mask/test