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

Introduce the GCPDefinition #476

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mraspaud
Copy link
Member

@mraspaud mraspaud commented Nov 16, 2022

Add a new class called GCPDefinition (inherits from SwathDefinition) which provides access to the underlying gcps.

Use cases:

  • faster loading of coarse lon/lats (for the boundary)

  • if you want to write data with tiepoints

  • interpolate a fraction of what is needed.

  • Closes #xxxx

  • Tests added

  • Tests passed

  • Passes git diff origin/main **/*py | flake8 --diff

  • Fully documented

@mraspaud mraspaud self-assigned this Nov 16, 2022
@mraspaud
Copy link
Member Author

@BENR0 ^

@codecov
Copy link

codecov bot commented Nov 16, 2022

Codecov Report

Merging #476 (71a019c) into main (1268d90) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #476      +/-   ##
==========================================
+ Coverage   94.28%   94.29%   +0.01%     
==========================================
  Files          69       70       +1     
  Lines       12394    12417      +23     
==========================================
+ Hits        11686    11709      +23     
  Misses        708      708              
Flag Coverage Δ
unittests 94.29% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pyresample/geometry.py 87.22% <100.00%> (+0.05%) ⬆️
pyresample/test/test_gcp_def.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 93.802% when pulling 71a019c on mraspaud:feature-tiepoint-def into 1268d90 on pytroll:main.

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.

I'm not sure I understand all of the use cases for GCPs, but I thought they were a sparse version representing something much larger. How does this work here with your definition? Put another way, I thought the user would give GCPs and like an overall shape of the area and that would be used to generate the full lons/lats on the fly. Where is get_coarse_bbox_lonlats going to be used? It isn't used currently, right? How useful is it when we have access to the full lons/lats already? How will the user control whether "coarse" bounding boxes are good enough for their use case?

@mraspaud
Copy link
Member Author

I'm not sure I understand all of the use cases for GCPs, but I thought they were a sparse version representing something much larger. How does this work here with your definition? Put another way, I thought the user would give GCPs and like an overall shape of the area and that would be used to generate the full lons/lats on the fly. Where is get_coarse_bbox_lonlats going to be used? It isn't used currently, right? How useful is it when we have access to the full lons/lats already? How will the user control whether "coarse" bounding boxes are good enough for their use case?

Yes, very good questions indeed.

  1. This is just a proof of concept when testing SwathDefinition html representations (for speed) so it's not really used anywhere yet.
  2. Indeed, we could make the GCPDefinition have interpolation functions. However a problem is how would it know what interpolation scheme to use? One solution would be a callback to the interpolation function for example.
  3. The coarse bbox is basically just a quick test for now. I have no idea how we could quantify the quality of that, or fitness to give use case...

@BENR0
Copy link
Contributor

BENR0 commented Nov 17, 2022

I think another use case that was mentioned by @mraspaud was to be able to write geotiffs with only tiepoints.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

None yet

4 participants