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

Handle bilinear caching in Pyresample #1406

Merged
merged 21 commits into from
Nov 16, 2020
Merged

Handle bilinear caching in Pyresample #1406

merged 21 commits into from
Nov 16, 2020

Conversation

pnuu
Copy link
Member

@pnuu pnuu commented Oct 18, 2020 •
edited
Loading

This is the other half of moving bilinear resampler caching to Pyresample, the other bit is here. The PR in Pyresample needs to be merged and released, then retrigger the tests here and make a new Satpy release ASAP to have a matching features available.

  • Tests added
  • Passes flake8 satpy

@pnuu pnuu added enhancement code enhancements, features, improvements component:resampling labels Oct 18, 2020
@pnuu pnuu self-assigned this Oct 18, 2020
@pnuu pnuu added the blocked Waiting on an external action (eg release of other package) label Oct 18, 2020
@pnuu pnuu mentioned this pull request Oct 19, 2020
Merged
4 tasks
@pnuu pnuu requested review from mraspaud and djhoese October 19, 2020 12:43
@pnuu pnuu marked this pull request as ready for review October 19, 2020 12:43
mraspaud
mraspaud approved these changes Oct 23, 2020
View reviewed changes
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
Copy link
Member

This needs a new pyresample release, right?

djhoese
djhoese reviewed Oct 27, 2020
View reviewed changes
satpy/resample.py Outdated
try:
from pyresample.gradient import GradientSearchResampler
except ImportError:
warnings.warn('Gradient search resampler not available. Maybe missing `shapely`?.')
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to do this check inside some other function or to only show it when a user requests gradient search? It gets really annoying when most people probably aren't going to use the gradient search (for now at least).

Copy link
Member Author

Choose a reason for hiding this comment

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

Moved the warning to a place where the user tries to use it.

djhoese
djhoese approved these changes Oct 27, 2020
View reviewed changes
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.

Thanks!

@ghost
Copy link

ghost commented Nov 13, 2020 •
edited by ghost
Loading

Congratulations 🎉. DeepCode analyzed your code in 4.107 seconds and we found no issues. Enjoy a moment of no bugs ☀️.

👉 View analysis in DeepCode’s Dashboard | Configure the bot

@codecov
Copy link

codecov bot commented Nov 13, 2020 •
edited
Loading

Codecov Report

Merging #1406 (c12bc0c) into master (70f07f7) will decrease coverage by 0.01%.
The diff coverage is 74.19%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1406      +/-   ##
==========================================
- Coverage   90.66%   90.64%   -0.02%     
==========================================
  Files         236      236              
  Lines       33958    33940      -18     
==========================================
- Hits        30787    30766      -21     
- Misses       3171     3174       +3
Impacted Files Coverage Δ
satpy/resample.py 89.69% <52.94%> (-1.09%) ⬇️
satpy/tests/test_resample.py 99.31% <100.00%> (+0.47%) ⬆️

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 70f07f7...c12bc0c. Read the comment docs.

@coveralls
Copy link

coveralls commented Nov 13, 2020 •
edited
Loading

Coverage Status

Coverage decreased (-0.01%) to 90.648% when pulling c12bc0c on pnuu:feature-save_bil_info-in-resampler into 70f07f7 on pytroll:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.08%) to 90.647% when pulling 8cfc469 on pnuu:feature-save_bil_info-in-resampler into 12917ab on pytroll:master.

@pnuu
Copy link
Member Author

pnuu commented Nov 13, 2020

I added a quick'n'dirty fix to yaml_reader tests that failed due to pytroll/pyresample#290 making area_extent immutable.

sfinkens
sfinkens approved these changes Nov 16, 2020
View reviewed changes
Copy link
Member

@sfinkens sfinkens left a comment

Choose a reason for hiding this comment

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

Nice job!

@mraspaud mraspaud merged commit 02d0d9b into pytroll:master Nov 16, 2020
@pnuu pnuu deleted the feature-save_bil_info-in-resampler branch November 16, 2020 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mraspaud mraspaud mraspaud approved these changes

@djhoese djhoese djhoese approved these changes

@sfinkens sfinkens sfinkens approved these changes

@adybbroe adybbroe Awaiting requested review from adybbroe

Assignees

@pnuu pnuu

Labels
blocked Waiting on an external action (eg release of other package) component:resampling enhancement code enhancements, features, improvements
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@pnuu @mraspaud @coveralls @djhoese @sfinkens