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 Scene crop so new areas are consistent with resolution #406

Merged
merged 3 commits into from Sep 7, 2018

Conversation

djhoese
Copy link
Member

@djhoese djhoese commented Sep 4, 2018

As described in #336, there are cases when the crop method can produce undesired results. If crop is used before resampling (a perfectly valid use case) and the input datasets are made up of different resolutions then the resulting areas would each have different shapes/dimensions. Or at least they could. This PR makes them always cover the same geographic area.

I'm not happy with the solution I had to end up with but it works for my simple test cases with ABI so far. I was hoping I could come up with one area definition for the lowest resolution area and use that as the target area for the other croppings. Due to floating point issues and other "rounding"/fudging the results aren't consistent. My solution simply multiplies the slicing indexes by a certain factor assuming that the input datasets are all a round factor of each other (1x, 2x, 4x). Due to the rounding issues in pyresample I'm not sure there is a better solution but welcome other ideas.

I still need to add tests for these specific cases.

@djhoese djhoese added this to the v0.10 milestone Sep 4, 2018
@djhoese djhoese self-assigned this Sep 4, 2018
@coveralls
Copy link

coveralls commented Sep 4, 2018

Coverage Status

Coverage decreased (-0.006%) to 71.403% when pulling 18ab961 on djhoese:bugfix-crop into 34dd5dc on pytroll:master.

@codecov
Copy link

codecov bot commented Sep 5, 2018

Codecov Report

Merging #406 into master will increase coverage by 0.01%.
The diff coverage is 91.89%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #406      +/-   ##
==========================================
+ Coverage    71.4%   71.42%   +0.01%     
==========================================
  Files         126      126              
  Lines       16428    16449      +21     
==========================================
+ Hits        11731    11749      +18     
- Misses       4697     4700       +3
Impacted Files Coverage Δ
satpy/tests/test_scene.py 99.31% <100%> (ø) ⬆️
satpy/scene.py 84.45% <89.65%> (-0.12%) ⬇️

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 34dd5dc...18ab961. Read the comment docs.

@djhoese djhoese changed the title [WIP] Fix Scene crop so new areas are consistent with resolution Fix Scene crop so new areas are consistent with resolution Sep 5, 2018
@djhoese
Copy link
Member Author

djhoese commented Sep 5, 2018

This should probably be cherry picked to a 0.9.x release too.

@djhoese djhoese mentioned this pull request Sep 5, 2018
3 tasks
@djhoese djhoese merged commit 18e67b3 into pytroll:master Sep 7, 2018
@djhoese djhoese deleted the bugfix-crop branch September 7, 2018 16:52
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.

Scene crop does not compare all dataset areas
2 participants