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

Use the new levels module. #1669

Merged
merged 1 commit into from
Dec 3, 2019
Merged

Use the new levels module. #1669

merged 1 commit into from
Dec 3, 2019

Conversation

ttung
Copy link
Collaborator

@ttung ttung commented Nov 22, 2019

Adjusts all the filters to use levels instead of clip/preserve_float_range. clip_method is still supported though deprecated. If both are provided, an exception is thrown. If clip_method by itself is provided, then a deprecation warning is printed and the clip_method is translated to the appropriate levels_method.

Depends on #1666, #1668

Test plan: this is an -alltest branch since it affects so much code. if goes green, we should presumably be good. :)

@ttung ttung requested a review from shanaxel42 November 22, 2019 07:45
@codecov-io
Copy link

codecov-io commented Nov 22, 2019

Codecov Report

Merging #1669 into master will decrease coverage by 0.09%.
The diff coverage is 84.88%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #1669     +/-   ##
========================================
- Coverage    90.2%   90.1%   -0.1%     
========================================
  Files         231     231             
  Lines        8808    8835     +27     
========================================
+ Hits         7945    7961     +16     
- Misses        863     874     +11
Impacted Files Coverage Δ
starfish/core/types/_constants.py 98.43% <ø> (ø) ⬆️
starfish/core/image/Filter/gaussian_high_pass.py 100% <100%> (ø) ⬆️
starfish/core/image/Filter/mean_high_pass.py 100% <100%> (ø) ⬆️
...core/image/Filter/richardson_lucy_deconvolution.py 95.45% <100%> (ø) ⬆️
starfish/core/image/Filter/gaussian_low_pass.py 100% <100%> (ø) ⬆️
starfish/core/image/Filter/map.py 100% <100%> (ø) ⬆️
starfish/core/image/Filter/linear_unmixing.py 100% <100%> (ø) ⬆️
starfish/core/image/Filter/laplace.py 100% <100%> (ø) ⬆️
starfish/core/image/Filter/bandpass.py 96.42% <100%> (ø) ⬆️
starfish/core/image/Filter/white_tophat.py 100% <100%> (ø) ⬆️
... and 3 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 e1375df...6e2a100. Read the comment docs.

@ttung ttung force-pushed the tonytung-use-levels-alltest branch 2 times, most recently from d86fdd3 to da6a80c Compare November 22, 2019 08:56
@ttung ttung force-pushed the tonytung-use-levels-base branch from e4c18d3 to d5a752f Compare November 22, 2019 17:56
@ttung ttung force-pushed the tonytung-use-levels-alltest branch from da6a80c to 89bc8ed Compare November 22, 2019 17:56
@ttung ttung changed the base branch from tonytung-use-levels-base to tonytung-levels November 22, 2019 17:57
@ttung ttung force-pushed the tonytung-use-levels-alltest branch from 89bc8ed to 17a4d5f Compare November 22, 2019 17:57
Adjusts all the filters to use levels instead of clip/preserve_float_range.  `clip_method` is still supported though deprecated.  If both are provided, an exception is thrown.  If `clip_method` by itself is provided, then a deprecation warning is printed and the `clip_method` is translated to the appropriate `levels_method`.

Depends on #1666, #1668

Test plan: this is an -alltest branch since it affects so much code.  if goes green, we should presumably be good. :)
@ttung ttung force-pushed the tonytung-use-levels-alltest branch from 17a4d5f to 6e2a100 Compare December 3, 2019 19:38
@ttung ttung changed the base branch from tonytung-levels to master December 3, 2019 19:39
@ttung ttung merged commit 2e61ae0 into master Dec 3, 2019
@ttung ttung deleted the tonytung-use-levels-alltest branch December 3, 2019 20:39
ttung pushed a commit that referenced this pull request Dec 3, 2019
#1652 replaced a block of code that scales the intensity of the image by the max, with a reduce operation that does Clip.SCALE_BY_IMAGE.  As I discovered later, Clip.SCALE_BY_IMAGE does not do what the documentation says it does, and in fact, it only scales by the max intensity if the max intensity is > 1.0.  That means it's not a refactor and actually affected the output.

Levels.SCALE_BY_IMAGE, however, does what we want here and can do what we originally had here.

Depends on #1669
Test plan: pretty much none.
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