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

Create an all-purpose ImageStack factory #1348

Merged
merged 1 commit into from
May 22, 2019
Merged

Create an all-purpose ImageStack factory #1348

merged 1 commit into from
May 22, 2019

Conversation

ttung
Copy link
Collaborator

@ttung ttung commented May 14, 2019

  1. Create a LocationAwareFetchedTile class, which is like FetchedTile, but is explicitly aware of its location in 5D space.
  2. Create an all_purpose.imagestack_factory method that produces an ImageStack with the provided coordinate ranges.
  3. Fix existing tests that did not deal with coordinates properly.
  4. Fix imagestack_test_utils.py::verify_physical_coordinates, which used zplane as an index rather than a value. In the case of labeled images, this makes a difference.

Test plan: make -j all

@ttung ttung requested review from ambrosejcarr and shanaxel42 May 14, 2019 22:52
@codecov-io
Copy link

codecov-io commented May 14, 2019

Codecov Report

Merging #1348 into master will decrease coverage by 1.22%.
The diff coverage is 98.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1348      +/-   ##
==========================================
- Coverage   90.42%   89.19%   -1.23%     
==========================================
  Files         216      148      -68     
  Lines        8240     5368    -2872     
==========================================
- Hits         7451     4788    -2663     
+ Misses        789      580     -209
Impacted Files Coverage Δ
...fish/core/imagestack/test/factories/all_purpose.py 100% <100%> (ø)
...ish/core/imagestack/test/factories/unique_tiles.py 96.55% <88.88%> (-1.13%) ⬇️
starfish/core/experiment/test/test_experiment.py
starfish/core/recipe/test/test_runnable.py
starfish/core/spacetx_format/test/test_validate.py
starfish/core/imagestack/test/test_export.py
...table/test/test_stack_intensity_table_reshaping.py
...rfish/core/intensity_table/test/test_to_mermaid.py
starfish/core/test/factories.py
...tarfish/core/image/_filter/test/test_call_bases.py
... and 59 more

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 6a2a2e3...bf8ddd8. Read the comment docs.

@ttung ttung force-pushed the tonytung-factory branch 6 times, most recently from fd907c3 to 22d4329 Compare May 20, 2019 21:42
@@ -41,6 +40,8 @@ def verify_physical_coordinates(stack: ImageStack,
# If zplane provided, test expected_z_coordinates on specific plane.
# Else just test expected_z_coordinates on entire array
if zplane is not None:
assert np.isclose(stack.xarray[Coordinates.Z.value][zplane], expected_z_coordinates)
assert np.isclose(
stack.xarray.sel({Axes.ZPLANE.value: zplane})[Coordinates.Z.value],
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think you need to do the sel, stack.xarray[Coordinates.Z.value].values should give you the array

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't want the array. I want a singular value for the plane.

@ttung ttung force-pushed the tonytung-factory branch 2 times, most recently from dfc8d40 to fcd2051 Compare May 21, 2019 18:35
1. Create a `LocationAwareFetchedTile` class, which is like `FetchedTile`, but is explicitly aware of its location in 5D space.
2. Create an all_purpose.imagestack_factory method that produces an ImageStack with the provided coordinate ranges.
3. Fix existing tests that did not deal with coordinates properly.
4. Fix `imagestack_test_utils.py::verify_physical_coordinates`, which used zplane as an index rather than a value.  In the case of labeled images, this makes a difference.

Test plan: `make -j all`
@ttung ttung force-pushed the tonytung-factory branch from fcd2051 to bf8ddd8 Compare May 21, 2019 18:52
@ttung ttung merged commit 8af3e83 into master May 22, 2019
@ttung ttung deleted the tonytung-factory branch May 22, 2019 23:00
@ttung ttung restored the tonytung-factory branch May 23, 2019 18:13
@ttung ttung deleted the tonytung-factory branch May 23, 2019 18:14
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.

3 participants