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

Factorize and improve modis reader's interpolation #802

Merged
merged 3 commits into from Jun 10, 2019

Conversation

mraspaud
Copy link
Member

@mraspaud mraspaud commented Jun 3, 2019

This avoids recomputation of parallel datasets that are anyway needed for
the interpolation.

  • Closes #xxxx
  • Tests added and test suite added to parent suite
  • Tests passed
  • Passes flake8 satpy
  • Fully documented

This avoids recomputation of parallel dataset that are anyway needed for 
the interpolation.
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.

Should these interpolation functions be delayed so that we aren't actually wasting memory by caching them?

@mraspaud
Copy link
Member Author

mraspaud commented Jun 3, 2019

From what I can see, the interpolation is dask-based, so I don't think a lot of memory is used

@djhoese
Copy link
Member

djhoese commented Jun 3, 2019

Ah looks like you are right, nevermind then. LGTM.

@djhoese
Copy link
Member

djhoese commented Jun 6, 2019

@mraspaud Is there anything else you are hoping to add to this PR or is it ready for review (it's still a draft currently)?

@mraspaud mraspaud marked this pull request as ready for review June 10, 2019 07:28
@mraspaud
Copy link
Member Author

It's ready for review now

@djhoese
Copy link
Member

djhoese commented Jun 10, 2019

Any idea how we get the travis tests to start up?

@mraspaud mraspaud closed this Jun 10, 2019
@mraspaud mraspaud reopened this Jun 10, 2019
@mraspaud
Copy link
Member Author

closing and reopening :(

@codecov
Copy link

codecov bot commented Jun 10, 2019

Codecov Report

Merging #802 into master will increase coverage by 0.79%.
The diff coverage is 82.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #802      +/-   ##
==========================================
+ Coverage   82.26%   83.06%   +0.79%     
==========================================
  Files         159      163       +4     
  Lines       22998    23592     +594     
==========================================
+ Hits        18920    19596     +676     
+ Misses       4078     3996      -82
Impacted Files Coverage Δ
satpy/tests/reader_tests/test_modis_l2.py 97.43% <100%> (+0.02%) ⬆️
satpy/composites/__init__.py 70.55% <50%> (+1.31%) ⬆️
satpy/readers/hdfeos_base.py 88.48% <88.88%> (+9.09%) ⬆️
satpy/readers/aapp_l1b.py 13.65% <0%> (-0.21%) ⬇️
satpy/writers/utils.py 100% <0%> (ø)
satpy/tests/reader_tests/test_vaisala_gld360.py 96% <0%> (ø)
satpy/tests/writer_tests/test_utils.py 93.33% <0%> (ø)
satpy/readers/vaisala_gld360.py 90% <0%> (ø)
satpy/tests/test_scene.py 99.45% <0%> (ø) ⬆️
satpy/tests/reader_tests/test_virr_l1b.py 97.19% <0%> (+0.02%) ⬆️
... and 14 more

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 6f1f451...778f078. Read the comment docs.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.8%) to 83.062% when pulling 778f078 on mraspaud:fix-modis-interpolation into 6f1f451 on pytroll:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.8%) to 83.062% when pulling 778f078 on mraspaud:fix-modis-interpolation into 6f1f451 on pytroll:master.

@mraspaud mraspaud added this to the v0.16 milestone Jun 10, 2019
@mraspaud mraspaud merged commit a5733f3 into pytroll:master Jun 10, 2019
@mraspaud mraspaud deleted the fix-modis-interpolation branch June 10, 2019 19: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

3 participants