-
Notifications
You must be signed in to change notification settings - Fork 443
Use PyProj instead of Cartopy for internal coordinate transforms and rename crs coordinate #1483
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
Conversation
89a5378 to
85cffcd
Compare
|
Just fixed up the rebase of this on latest master after the changes of #1490, but the todo list above remains. |
85cffcd to
011f45a
Compare
|
I looked at the list on Python 3, and API wise I think we'll be fine. However, they are dropping older versions of PROJ, and doing enough that I think the right approach is to make our PyProj dependency right now |
46873e6 to
be31c7d
Compare
|
The reason for the test failures is that before for...CartoPy reasons...we were using Before updating all the test values, I was able to make the tests pass by hacking the test metadata to specify a WGS84 spheroid, so that's clearly the issue. Further evidence that shifting over to using PyProj directly for calcs is a good thing to do. |
be31c7d to
3fdb776
Compare
|
Bumping our minimum pyproj to 2.3 was enough to make tests pass for me locally. Assuming that holds on CI, let's just go with that (released Aug 2019) rather than trying to do any special work-arounds. |
3fdb776 to
588784c
Compare
dcamron
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like these changes, and didn't find any other issues myself!
8272a02 to
cab7bd9
Compare
…rename crs coordinate
Was accidentally trying to upload coverage only when we ran without it.
Makes it easier to debug issues while PR is still in development.
Necessary because we are now properly using the Earth ellipsoid that is present in the data for lat/lon projection. Previously, we were using CartoPy's PlateCarree, which doesn't support globes--thus when we got the spheroid we used the CartoPy default, which is WGS84. Was able to reproduce previous test values by overriding data ellipsoid to specify WGS84. Updating the test values is better long-term than putting a WGS84 hack into the data fixture.
Apparently these have been changed for security reasons: https://github.blog/changelog/2020-10-01-github-actions-deprecating-set-env-and-add-path-commands/
cab7bd9 to
038872e
Compare
038872e to
ee2117d
Compare
Description Of Changes
Implements the changes discussed in #1455 (comment). Part of this was changing the API for
lat_lon_grid_deltasto take the PyProjGeoddirectly rather than by constructor kwargs (makes it easier to handle fromgrid_deltas_from_dataarray, which extracts theGeodfrom the PyProjCRS).Marking as draft/work-in-progress for now since the following have yet to be done:
azimuth_range_to_lat_lonand any otherGeoduses to takegeodarg rather thankwargsto construct newGeodpyproj >= 2.2a required dependency (unmodified as of now)If someone wanted to take these items on to speed up the process, that would be great! Otherwise, I'll loop back to this after the xarray stuff for 1.0.
Checklist