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 EWA resampling tests not properly testing caching #831

Merged
merged 2 commits into from
Jun 26, 2019

Conversation

djhoese
Copy link
Member

@djhoese djhoese commented Jun 26, 2019

This was first noticed by @zxdawn in #751. Starting with dask 2.0+ these tests failed. The main reason is that this test was not testing what it was supposed to be testing after changes a long time ago to resample.py. The other reason is that in dask 2.0+, any map_blocks function is called with fake/empty input data to check what array type object is going to be returned. This meant multiple calls to ll2cr when previously there were only calls for actual data calculations.

So this PR fixes:

  1. The EWA tests so that they actually make sure things were cached.
  2. Resampler caching so that hashing of resample_kwargs results in proper equality checks, meaning resampler's that should be re-used are reused.
  3. Fix geoviews conversion methods only issuing a warning if geoviews was not imported (FYI @BENR0) even though nothing could be done if it failed to import.
  • Tests added and test suite added to parent suite
  • Tests passed
  • Passes flake8 satpy

@djhoese
Copy link
Member Author

djhoese commented Jun 26, 2019

FYI See dask/dask#5007 for my questions/concerns about this dask metadata discovery.

@codecov
Copy link

codecov bot commented Jun 26, 2019

Codecov Report

Merging #831 into master will increase coverage by 0.05%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #831      +/-   ##
=========================================
+ Coverage   83.84%   83.9%   +0.05%     
=========================================
  Files         167     167              
  Lines       24441   24445       +4     
=========================================
+ Hits        20492   20510      +18     
+ Misses       3949    3935      -14
Impacted Files Coverage Δ
satpy/scene.py 90.65% <100%> (+0.63%) ⬆️
satpy/tests/test_resample.py 85.89% <100%> (+0.37%) ⬆️
satpy/resample.py 89.38% <100%> (+2.15%) ⬆️
satpy/readers/avhrr_l1b_gaclac.py 90.14% <0%> (-1.41%) ⬇️
satpy/composites/__init__.py 74.64% <0%> (+0.25%) ⬆️

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 fa6f33e...bfeacd0. Read the comment docs.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) to 83.903% when pulling bfeacd0 on djhoese:bugfix-ewa-tests into fa6f33e on pytroll:master.

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

@djhoese djhoese merged commit 2654cf3 into pytroll:master Jun 26, 2019
@djhoese djhoese deleted the bugfix-ewa-tests branch June 26, 2019 18:59
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.

3 participants