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 ratio sharpening not sharing invalid mask between bands #2262

Merged
merged 1 commit into from Nov 4, 2022

Conversation

djhoese
Copy link
Member

@djhoese djhoese commented Nov 4, 2022

Fixes a bug introduced in #2240 where I refactored the duplicate masking behavior because it was already handled in the parent class, GenericCompositor. However, I didn't change the default for the ratio sharpening class to enable this behavior in the parent class so it wasn't being done at all. The tests were not complex enough to pick up this mistake but have been updated in this PR.

The behavior I'm talking about is where a mask is created of the NaN locations in each band, merged, and then applied to all the bands. Without this, you get strange colors where one band has missing data but another doesn't. This is especially necessary on the edges of resampled data where different resolution may "stretch" further geographically than other bands.

@djhoese
Copy link
Member Author

djhoese commented Nov 4, 2022

When I run asv on my own machine (while I'm doing other things) for the last 4 commits where this PR is the last entry, I get this for peakmem on the AHI true_color load test:

image

So not much of a change. Time:

image

It may look like it goes up, but those other commits weren't even related to these changes so this is just system variance from my point of view. I think this is safe to merge in that regard.

I'm still tracking down another issue that @kathys identified in ssec/polar2grid#558 regarding quality/sharpness of the images, but I'm not sure it is related to these refactorings. I should know more in the next hour or so.

@codecov
Copy link

codecov bot commented Nov 4, 2022

Codecov Report

Merging #2262 (d620c12) into main (05419d7) will increase coverage by 0.06%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2262      +/-   ##
==========================================
+ Coverage   94.12%   94.19%   +0.06%     
==========================================
  Files         295      295              
  Lines       45376    45379       +3     
==========================================
+ Hits        42712    42746      +34     
+ Misses       2664     2633      -31     
Flag Coverage Δ
behaviourtests 4.66% <0.00%> (-0.01%) ⬇️
unittests 94.84% <100.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
satpy/composites/__init__.py 90.25% <ø> (-0.02%) ⬇️
satpy/tests/test_composites.py 100.00% <100.00%> (ø)
satpy/scene.py 93.37% <0.00%> (+0.15%) ⬆️
satpy/resample.py 79.56% <0.00%> (+0.16%) ⬆️
satpy/writers/mitiff.py 92.45% <0.00%> (+0.23%) ⬆️
satpy/readers/viirs_sdr.py 86.94% <0.00%> (+0.26%) ⬆️
satpy/tests/test_config.py 97.18% <0.00%> (+0.40%) ⬆️
satpy/readers/eps_l1b.py 83.62% <0.00%> (+0.43%) ⬆️
satpy/readers/seviri_l1b_native_hdr.py 100.00% <0.00%> (+0.57%) ⬆️
satpy/readers/mviri_l1b_fiduceo_nc.py 100.00% <0.00%> (+0.74%) ⬆️
... and 11 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@djhoese
Copy link
Member Author

djhoese commented Nov 4, 2022

CI is having a rough day today. First it was a mamba hiccup, now it is a coveralls connection hiccup. 🤞

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0003%) to 94.794% when pulling d620c12 on djhoese:bugfix-ratiosharpen-again into 05419d7 on pytroll:main.

@djhoese
Copy link
Member Author

djhoese commented Nov 4, 2022

Merging given the simplicity of the fix, the fact that I broke it originally, and the issues it is causing for me downstream. Slap my wrist if you must. 📏

@djhoese djhoese merged commit a78ff28 into pytroll:main Nov 4, 2022
@djhoese djhoese deleted the bugfix-ratiosharpen-again branch November 4, 2022 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants