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

Ignore alpha when adding luminance in Sandwich compositor #2080

Merged
merged 11 commits into from Apr 6, 2022

Conversation

mherbertson
Copy link
Contributor

@mherbertson mherbertson commented Apr 4, 2022

This updates the Sandwich compositor to ignore the alpha band when modifying.

  • Tests added

@mherbertson mherbertson changed the title Ignore alpha when adding luminance Ignore alpha when adding luminance in Sandwich compositor Apr 4, 2022
Copy link
Member

@djhoese djhoese left a comment

Choose a reason for hiding this comment

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

Wow, what a simple solution. Nice job and thanks for making the PR!

Are there tests for this compositor already? Could they be modified or added onto to check for this fix?

@mherbertson
Copy link
Contributor Author

That's a good idea. I think the existing check actually just did the first three bands 'RGB'. So adding one to check the alpha stays intact makes sense. I'll see what I can come up with.

@mherbertson
Copy link
Contributor Author

Existing test updated to confirm alpha band remains intact.

satpy/tests/test_composites.py Outdated Show resolved Hide resolved
satpy/tests/test_composites.py Outdated Show resolved Hide resolved
satpy/tests/test_composites.py Outdated Show resolved Hide resolved
satpy/tests/test_composites.py Outdated Show resolved Hide resolved
satpy/tests/test_composites.py Outdated Show resolved Hide resolved
satpy/tests/test_composites.py Outdated Show resolved Hide resolved
satpy/tests/test_composites.py Outdated Show resolved Hide resolved
satpy/tests/test_composites.py Outdated Show resolved Hide resolved
satpy/tests/test_composites.py Outdated Show resolved Hide resolved
satpy/tests/test_composites.py Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Apr 6, 2022

Codecov Report

Merging #2080 (0a97b74) into main (683df59) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #2080   +/-   ##
=======================================
  Coverage   93.79%   93.79%           
=======================================
  Files         283      283           
  Lines       42281    42284    +3     
=======================================
+ Hits        39659    39662    +3     
  Misses       2622     2622           
Flag Coverage Δ
behaviourtests 4.73% <0.00%> (-0.01%) ⬇️
unittests 94.36% <100.00%> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
satpy/composites/__init__.py 89.92% <100.00%> (ø)
satpy/tests/test_composites.py 100.00% <100.00%> (ø)

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 683df59...0a97b74. Read the comment docs.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0004%) to 94.3% when pulling 0a97b74 on mherbertson:update-sandwich-compositor into 683df59 on pytroll:main.

@mherbertson
Copy link
Contributor Author

Not sure what to do about the CodeFactor failure.

@mraspaud
Copy link
Member

mraspaud commented Apr 6, 2022

@mherbertson that usually means that some of the functions/classes in that modules are too convoluted, and should thus be refactored to smaller functions/classes. But in the changes you made, I don't really see how to make things simpler, do you?

@mherbertson
Copy link
Contributor Author

@mraspaud Thanks for the explanation! I don't think there's much I can do to simplify.

Copy link
Member

@mraspaud mraspaud left a comment

Choose a reason for hiding this comment

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

LGTM

@mraspaud mraspaud merged commit fd483d3 into pytroll:main Apr 6, 2022
@mherbertson mherbertson deleted the update-sandwich-compositor branch April 6, 2022 07:24
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

5 participants