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

Add no-op image_ready enhancement #2933

Merged
merged 4 commits into from
Oct 17, 2024

Conversation

mraspaud
Copy link
Member

This PR adds a no-op enhancement for data that is image-ready, ie where nothing needs to be done. The enhancement key (standard_name) is called image_ready.

I didn't add any test, as the function is not doing anything. But shout if you think I should do otherwise.

@mraspaud mraspaud added enhancement code enhancements, features, improvements component:enhancements labels Oct 16, 2024
@mraspaud mraspaud requested a review from pnuu October 16, 2024 16:05
@mraspaud mraspaud self-assigned this Oct 16, 2024
@mraspaud mraspaud requested a review from djhoese as a code owner October 16, 2024 16:05
@pnuu
Copy link
Member

pnuu commented Oct 16, 2024

I'm curious to see if there's a performance boost to what I've had before. I've only added a crude stretch from 0 to 1 for the composites using DayNightCompositor as enhancement. Will test tomorrow.

Copy link

codecov bot commented Oct 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.08%. Comparing base (ba30733) to head (b408e65).
Report is 6 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2933   +/-   ##
=======================================
  Coverage   96.08%   96.08%           
=======================================
  Files         375      375           
  Lines       54578    54586    +8     
=======================================
+ Hits        52441    52449    +8     
  Misses       2137     2137           
Flag Coverage Δ
behaviourtests 3.98% <11.11%> (+<0.01%) ⬆️
unittests 96.18% <100.00%> (+<0.01%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mraspaud
Copy link
Member Author

I'm curious to see if there's a performance boost to what I've had before. I've only added a crude stretch from 0 to 1 for the composites using DayNightCompositor as enhancement. Will test tomorrow.

IIRC, the stretch function is "smart" and doesn't do anything if min and max are respectively 0 and 1. But the method in this PR is cleaner I think.

@coveralls
Copy link

coveralls commented Oct 16, 2024

Pull Request Test Coverage Report for Build 11379521585

Details

  • 9 of 9 (100.0%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.001%) to 96.181%

Totals Coverage Status
Change from base Build 11365975625: 0.001%
Covered Lines: 52685
Relevant Lines: 54777

💛 - Coveralls

@pnuu
Copy link
Member

pnuu commented Oct 17, 2024

I'm curious to see if there's a performance boost to what I've had before. I've only added a crude stretch from 0 to 1 for the composites using DayNightCompositor as enhancement. Will test tomorrow.

IIRC, the stretch function is "smart" and doesn't do anything if min and max are respectively 0 and 1. But the method in this PR is cleaner I think.

You were right, no difference in performance.

Copy link
Member

@pnuu pnuu 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 5a8b6cc into pytroll:main Oct 17, 2024
18 checks passed
@mraspaud mraspaud deleted the feature-no-op-enhancement branch October 17, 2024 07:06
@ameraner
Copy link
Member

What is the difference between using this and a simple operations: []?

@mraspaud
Copy link
Member Author

IIRC, if operations is empty, a default enhancement is applied. This block any enhancement.

@gerritholl
Copy link
Member

gerritholl commented Oct 17, 2024

As far as I remember, operations: [] means no enhancement is applied. If operations is not defined at all, the default enhancement is applied. It is possible that I am mistaken.

@mraspaud
Copy link
Member Author

Ok, I can check, but either way, isn't it better to have it explicit?

@ameraner
Copy link
Member

ameraner commented Oct 17, 2024

Exactly, as Gerrit is saying, operations: [] means no enhancement, I use it explicitly when I don't want the default enhancement to kick in, e.g. for a BackgroundCompositor (see for example these usages). This is actually the indicated way in the documentation, see the warning box here: https://satpy.readthedocs.io/en/stable/composites.html#enhancing-the-images

@gerritholl
Copy link
Member

Ok, I can check, but either way, isn't it better to have it explicit?

Probably yes, but then please also update the documentation in the warning box that Andrea linked :)

@mraspaud
Copy link
Member Author

Will do!

@mraspaud mraspaud mentioned this pull request Oct 17, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:enhancements enhancement code enhancements, features, improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants