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 CRS object being recreated when adding CRS coordinate #915

Merged
merged 2 commits into from
Sep 27, 2019

Conversation

djhoese
Copy link
Member

@djhoese djhoese commented Sep 26, 2019

While fixing other CRS issues with trollimage and pyresample, I noticed that the CRS coordinate being added by Satpy is always created from the proj_str attribute of the AreaDefinition. This is an inaccurate/lossy way of creating a CRS definition. This PR makes it so we just copy the CRS object from the AreaDefinition if it exists. When this functionality was originally added the AreaDefinition didn't have a 'crs' property so creating from the proj_str made a little more sense. Now that we have a 'crs' object in newer versions of pyresample we should use it.

NOTE: I had uninstalled the pre-commit checks so I have not updated resample.py if it needs it.

  • Tests added and test suite added to parent suite
  • Tests passed
  • Passes flake8 satpy

@djhoese djhoese added the bug label Sep 26, 2019
@djhoese djhoese requested a review from mraspaud September 26, 2019 17:12
@djhoese djhoese requested a review from pnuu as a code owner September 26, 2019 17:12
@djhoese djhoese self-assigned this Sep 26, 2019
@coveralls
Copy link

Coverage Status

Coverage increased (+0.005%) to 85.289% when pulling 1b022f2 on djhoese:bugfix-resample-crs-coord into 35739e4 on pytroll:master.

@codecov
Copy link

codecov bot commented Sep 27, 2019

Codecov Report

Merging #915 into master will increase coverage by 0.16%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #915      +/-   ##
==========================================
+ Coverage   85.28%   85.44%   +0.16%     
==========================================
  Files         174      174              
  Lines       25822    25875      +53     
==========================================
+ Hits        22022    22110      +88     
+ Misses       3800     3765      -35
Impacted Files Coverage Δ
satpy/tests/test_resample.py 98.37% <100%> (+0.04%) ⬆️
satpy/resample.py 92.97% <100%> (+0.02%) ⬆️
satpy/scene.py 90.65% <0%> (+0.17%) ⬆️
satpy/readers/avhrr_l1b_gaclac.py 91.54% <0%> (+1.4%) ⬆️
satpy/tests/reader_tests/test_nwcsaf_nc.py 95.89% <0%> (+5.41%) ⬆️
satpy/readers/nwcsaf_nc.py 55.15% <0%> (+20.18%) ⬆️

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 35739e4...a7fd0cd. Read the comment docs.

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 0965d97 into pytroll:master Sep 27, 2019
@djhoese djhoese deleted the bugfix-resample-crs-coord branch September 27, 2019 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants