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

[easy] update requirements #1683

Merged
merged 1 commit into from
Dec 4, 2019
Merged

[easy] update requirements #1683

merged 1 commit into from
Dec 4, 2019

Conversation

ttung
Copy link
Collaborator

@ttung ttung commented Dec 2, 2019

No description provided.

@ttung ttung requested a review from shanaxel42 December 2, 2019 19:00
@codecov-io
Copy link

codecov-io commented Dec 2, 2019 •
edited
Loading

Codecov Report

Merging #1683 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1683   +/-   ##
=======================================
  Coverage   90.13%   90.13%           
=======================================
  Files         233      233           
  Lines        8870     8870           
=======================================
  Hits         7995     7995           
  Misses        875      875
Impacted Files Coverage Δ
...re/spots/DetectPixels/combine_adjacent_features.py 98.16% <100%> (ø) ⬆️

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 841d799...201dc4c. Read the comment docs.

@ttung ttung force-pushed the tonytung-up-reqs branch 2 times, most recently from 45cf775 to bd12941 Compare December 2, 2019 19:41
ttung
ttung commented Dec 2, 2019
View reviewed changes
starfish/core/spots/DetectPixels/combine_adjacent_features.py
@@ -212,7 +212,7 @@ def _calculate_mean_pixel_traces(

# the 0th pixel trace corresponds to background. If present, drop it.
try:
mean_pixel_traces = mean_pixel_traces.drop(0, dim=Features.AXIS)
mean_pixel_traces = mean_pixel_traces.drop_sel({Features.AXIS: 0})
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ambrosejcarr it looks like drop is being deprecated. I changed this to drop_sel, but there is no test coverage for this code. can you visually check this to see if it seems sane?

@ttung ttung force-pushed the tonytung-up-reqs branch 2 times, most recently from 39022b2 to 9bcb292 Compare December 3, 2019 20:39
@ttung ttung merged commit 8e45195 into master Dec 4, 2019
@ttung ttung deleted the tonytung-up-reqs branch December 4, 2019 23:13
@ambrosejcarr
Copy link
Member

ambrosejcarr commented Dec 5, 2019 via email

This was referenced Jan 10, 2020
Closed
Merged
ttung pushed a commit that referenced this pull request Jan 10, 2020
Move xarray to 0.14.1 (#1728)
ef698bb 
`DataArray.drop_sel` was added in 0.14.1.  The old API (removed in #1683) is being deprecated so we are want the new API.  However, that is only added in 0.14.1.

Fixes: #1726
Test plan: make -j fast
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

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
@ttung @codecov-io @ambrosejcarr @shanaxel42