Skip to content

Investigate antimeridian improvements and pyproj updates #645

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

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

djhoese
Copy link
Member

@djhoese djhoese commented Jan 30, 2025

I had these changes lying around and needed to get them out of the way. Maybe I'll get back to this soon...for now it is here for comment if people are really really bored.

  • Closes #xxxx
  • Tests added
  • Tests passed
  • Fully documented

@djhoese djhoese self-assigned this Jan 30, 2025
@djhoese djhoese force-pushed the feat-antimeridian-lonwrap branch 2 times, most recently from 70f477c to 559bd7c Compare February 17, 2025 21:22
@djhoese
Copy link
Member Author

djhoese commented Feb 17, 2025

Needs versioneer fixes from #648

@djhoese djhoese force-pushed the feat-antimeridian-lonwrap branch from 559bd7c to cc4cb05 Compare February 18, 2025 15:53
Copy link

codecov bot commented Feb 27, 2025

Codecov Report

Attention: Patch coverage is 98.48485% with 1 line in your changes missing coverage. Please review.

Project coverage is 93.98%. Comparing base (98f20a7) to head (2c4004b).
Report is 14 commits behind head on main.

Files with missing lines Patch % Lines
pyresample/test/test_kd_tree.py 85.71% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #645   +/-   ##
=======================================
  Coverage   93.98%   93.98%           
=======================================
  Files          86       86           
  Lines       13543    13534    -9     
=======================================
- Hits        12728    12720    -8     
+ Misses        815      814    -1     
Flag Coverage Δ
unittests 93.98% <98.48%> (+<0.01%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@coveralls
Copy link

coveralls commented Feb 27, 2025

Coverage Status

coverage: 93.68% (+0.003%) from 93.677%
when pulling 2c4004b on djhoese:feat-antimeridian-lonwrap
into 873a511 on pytroll:main.

@djhoese djhoese added enhancement backwards-incompatibility Causes backwards incompatibility or introduces a deprecation labels Mar 24, 2025
@djhoese
Copy link
Member Author

djhoese commented Mar 24, 2025

@mraspaud @pnuu I'm curious what you guys think about this PR and if I should split it up or go further or I don't know what. The main important change that I wanted was the DynamicAreaDefinition's change to use +lon_wrap=180 which allows extents to go beyond 180 longitude (0-360 essentially). However, that gets complicated because for that to be useful the nearest neighbor resampling also needs to support those coordinates which means removing the valid input/output index arrays. Then we get more complicated since there is base kdtree resampling (numpy-style), then an xarray-based/dask class used by satpy with caching, and an xarray/dask class with included caching in the "future" subpackage of pyresample.

Do I maybe switch this PR to be just the DynamicAreaDefinition changes? Then do another PR for the future class to drop valid input/output arrays? Then do another PR for the xarray-based class (and maybe the numpy functions) to drop valid input/output? I'm not sure.

This doesn't even begin to talk about my real desire which is to explore the XYZ transformation by pyproj changes after all of this (which I have working locally).

@pnuu
Copy link
Member

pnuu commented Mar 24, 2025

I'd say do the DynamicAreaDefinition first and the others later on.

Just a thought on the input/output array stuff perhaps be adapted based on the validity range of the coordinates? I don't remember exactly, but I think at the masking we would know whether the coordinates are from -180 to 180 or from 0 to 360. I'm not say not to remove them if it makes new things possible, just a thought.

@djhoese
Copy link
Member Author

djhoese commented Mar 24, 2025

For validity arrays, I think the bottom line, from what I can tell, is that in modern practices they are unnecessary. From the sound of it, most users are probably masking their geolocation already or Satpy is or the input files people are reading do. Or as a last resort the computations by pyproj will make them infinity or NaN.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backwards-incompatibility Causes backwards incompatibility or introduces a deprecation enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants