8000 [2/2] label image target assignment by ambrosejcarr · Pull Request #709 · spacetx/starfish · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[2/2] label image target assignment #709

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
Merged
merged 3 commits into from
Oct 19, 2018
Merged

[2/2] label image target assignment #709

merged 3 commits into from
Oct 19, 2018

Conversation

ambrosejcarr
Copy link
Member
@ambrosejcarr ambrosejcarr commented Oct 15, 2018 •
edited
Loading

This PR:

  • Implements TargetAssignment.Label to do assignment using a label image
  • Removes PointInPoly2D
  • Removes regional dependency
  • Updates dependent code & notebooks

Testing plan:

  • I wasn't really sure how to test this new code. I will need to think more. However, I did verify that things are working via some examinination of the ISS notebook.

ISS Image:

  • White: segmented "cells"
  • Red: spots that have not been assigned by TargetAssignment.Label
  • Blue: spots that were assigned by TargetAssignment.Label

This identifies an obvious problem with our segmentation approach, but assignment appears to be working.

Test code, can be run at end of ISS notebook:

import starfish
lab = starfish.spots.TargetAssignment.Label()

assigned = lab.run(label_image, decoded)

falls_inside_cells = assigned[assigned['cell_id'].astype(bool)]
falls_outside_cells = assigned[~assigned['cell_id'].astype(bool)]

image(seg._segmentation_instance.segmented > 0, size=20)
plt.scatter(falls_inside_cells.x, falls_inside_cells.y, alpha=1.0, s=6, c='b')
plt.scatter(falls_outside_cells.x, falls_outside_cells.y, alpha=0.5, c='r', s=6)

@ambrosejcarr
Copy link
Member Author
ambrosejcarr commented Oct 15, 2018 •
edited
Loading

Results of the above test code:
results

@ambrosejcarr ambrosejcarr changed the title Ajc label image target assignment [2/2] label image target assignment Oct 15, 2018
joshmoore
joshmoore reviewed Oct 16, 2018
View reviewed changes
REQUIREMENTS.txt.in
@@ -5,7 +5,6 @@ numpy != 1.13.0
pandas >= 0.23.4
pycodestyle<2.4.0
pytest>=3.6.3
regional
Copy link
Member

Choose a reason for hiding this comment

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

Until a make target is in place to keep everything in sync, either a josh-ping on requirement changes or a modification of environment.yml would be appreciated.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a way to automatically source the requirements in environment.yml from a file?

Copy link
Member

Choose a reason for hiding this comment

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

There is, but it likely changes the purpose of this. As it stands, the file reproduces what will be installed by using conda install -c bioconda starfish. If it sources requirements, it will match (far better) the current source code. I haven't found a way for bioconda's meta.yaml to just re-use some existing definition. I can add the generation code for bioconda to spacetx/starfish though. Thoughts on a make release target?

Copy link
Collaborator

Choose a reason for hiding this comment

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

What do you envision in a make release target?

Copy link
Member
@joshmoore joshmoore Oct 20, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

  • run extended check_requirements to sync conda
  • bump version number
  • commit, tag, push tag
  • add .dev to version number, commit, and push to master
  • build package and push to pypi
  • build docker and push to hub
  • generate and push commit to bioconda-recipes (and potentially open a PR)

Copy link
Member

Choose a reason for hiding this comment

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

migrated to #761

@ambrosejcarr ambrosejcarr force-pushed the ajc-label-image-target-assignment branch from 409cb0a to f4fe64d Compare October 17, 2018 15:03
@codecov-io
Copy link
codecov-io commented Oct 17, 2018 •
edited
Loading

Codecov Report

Merging #709 into ajc-segmentation-modernization will decrease coverage by 1.06%.
The diff coverage is 61.02%.

Impacted file tree graph

@@                        Coverage Diff                         @@
##           ajc-segmentation-modernization     #709      +/-   ##
==================================================================
- Coverage                           86.25%   85.18%   -1.07%     
==================================================================
  Files                                 123      115       -8     
  Lines                                4445     3956     -489     
==================================================================
- Hits                                 3834     3370     -464     
+ Misses                                611      586      -25
Impacted Files Coverage Δ
starfish/data/__init__.py 70.58% <ø> (ø) ⬆️
starfish/util/exec.py 83.33% <ø> (+4.76%) ⬆️
starfish/test/dataset_fixtures.py 52.33% <ø> (ø) ⬆️
starfish/spots/_detector/detect.py 96.61% <ø> (+1.02%) ⬆️
...arfish/test/full_pipelines/cli/test_merfish_cli.py 75% <ø> (ø) ⬆️
...rfish/test/full_pipelines/cli/test_dartfish_cli.py 100% <ø> (ø) ⬆️
starfish/test/full_pipelines/api/test_dartfish.py 100% <ø> (ø) ⬆️
...h/test/full_pipelines/cli/test_allen_smFISH_cli.py 75% <ø> (ø) ⬆️
starfish/test/full_pipelines/cli/test_iss.py 100% <ø> (ø) ⬆️
starfish/spots/_detector/local_max_peak_finder.py 0% <0%> (-32.61%) ⬇️
... and 27 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 d8430bf...ee99c02. Read the comment docs.

ttung
ttung approved these changes Oct 18, 2018
View reviewed changes
REQUIREMENTS.txt.in
@@ -5,7 +5,6 @@ numpy != 1.13.0
pandas >= 0.23.4
pycodestyle<2.4.0
pytest>=3.6.3
regional
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a way to automatically source the requirements in environment.yml from a file?

starfish/image/_segmentation/__init__.py
coordinates = r.coordinates
return [make_dict(id_, verts) for id_, verts in enumerate(coordinates)]
print(f"Writing label image to {args.output}")
imsave(args.output, label_image)
Copy link
Collaborator

Choose a reason for hiding this comment

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

A bit concerned that our output format for this stage is a numpy array...

Copy link
Member Author

Choose a reason for hiding this comment

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

I believe an array is the field standard. We could wrap this up in an xarray. The image should have x, y, and z, as well as corresponding coordinates.

Copy link
Member Author

Choose a reason for hiding this comment

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

This warrants discussion, but should be done in a follow-up PR. #724

@ambrosejcarr ambrosejcarr force-pushed the ajc-segmentation-modernization branch from 4c73dc1 to d8430bf Compare October 19, 2018 15:35
@ambrosejcarr ambrosejcarr force-pushed the ajc-label-image-target-assignment branch from 8d48ba3 to ee99c02 Compare October 19, 2018 16:25
@ambrosejcarr ambrosejcarr changed the base branch from ajc-segmentation-modernization to master October 19, 2018 16:55
@ambrosejcarr ambrosejcarr merged commit b1c20bc into master Oct 19, 2018
@ambrosejcarr ambrosejcarr deleted the ajc-label-image-target-assignment branch October 29, 2018 15:15
@joshmoore joshmoore mentioned this pull request Nov 1, 2018
Closed
10 tasks
@joshmoore
Copy link
Member

Note: rename propagated to docs in #886

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joshmoore joshmoore joshmoore left review comments

@ttung ttung ttung approved these changes

@shanaxel42 shanaxel42 Awaiting requested review from shanaxel42

@dganguli dganguli Awaiting requested review from dganguli

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

Successfully merging this pull request may close these issues.
4 participants
@ambrosejcarr @codecov-io @joshmoore @ttung
0