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

fix how spot_ids are handled by build_traces_sequential and Label._assign() #1872

Merged
merged 2 commits into from
May 7, 2020

Conversation

mattcai
Copy link
Contributor

@mattcai mattcai commented Apr 15, 2020

Two bug fixes:

  1. Made build_traces_sequential reassign unique spot_ids to every feature after concatenating PerImageSliceResults.
  2. Made Label._assign() explicitly label features by spot_id instead of row number, ensuring that labeling is accurate even when spot_ids do not match row numbers in an IntensityTable. Spot_ids must still be unique for accurate labeling though.

@codecov-io
Copy link

codecov-io commented Apr 15, 2020

Codecov Report

Merging #1872 into master will decrease coverage by 31.83%.
The diff coverage is 33.33%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #1872       +/-   ##
===========================================
- Coverage   87.77%   55.94%   -31.84%     
===========================================
  Files         168      168               
  Lines        5785     5786        +1     
===========================================
- Hits         5078     3237     -1841     
- Misses        707     2549     +1842     
Impacted Files Coverage Δ
starfish/core/spots/DecodeSpots/trace_builders.py 53.33% <0.00%> (-46.67%) ⬇️
starfish/core/spots/AssignTargets/label.py 91.17% <100.00%> (ø)
starfish/morphology.py 0.00% <0.00%> (-100.00%) ⬇️
...ish/core/imagestack/parser/tilefetcher/__init__.py 0.00% <0.00%> (-100.00%) ⬇️
...fish/core/imagestack/parser/tilefetcher/_parser.py 0.00% <0.00%> (-90.33%) ⬇️
starfish/test/full_pipelines/api/test_dartfish.py 19.67% <0.00%> (-80.33%) ⬇️
starfish/core/spots/DecodeSpots/util.py 20.40% <0.00%> (-79.60%) ⬇️
starfish/test/full_pipelines/api/test_iss_api.py 22.22% <0.00%> (-77.78%) ⬇️
...e/experiment/builder/test/factories/all_purpose.py 22.91% <0.00%> (-77.09%) ⬇️
starfish/core/starfish.py 0.00% <0.00%> (-76.93%) ⬇️
... and 87 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 cbdae57...0d30b88. Read the comment docs.

@mattcai mattcai force-pushed the mcai-sequentialtrace-spotids branch from d6bf00e to fe107e7 Compare April 15, 2020 23:45
…e after concatenating PerImageSliceResults. Also made Label._assign() explicitly label features by spot_id instead of row number, ensuring that labeling is accurate even when spot_ids do not match row_numbers in an IntensityTable. Spot_ids must still be unique for accurate labeling though.
@mattcai mattcai merged commit 7f15413 into master May 7, 2020
@mattcai mattcai deleted the mcai-sequentialtrace-spotids branch May 7, 2020 00:51
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