Skip to content

Fixing with Proj 9.8#2653

Draft
dopplershift wants to merge 3 commits intoSciTools:mainfrom
dopplershift:proj98
Draft

Fixing with Proj 9.8#2653
dopplershift wants to merge 3 commits intoSciTools:mainfrom
dopplershift:proj98

Conversation

@dopplershift
Copy link
Copy Markdown
Contributor

As noted in #2634, Proj 9.8 added support for properly handling ellipsoids to the equidistant cylindrical projection (OSGeo/PROJ#4656). This breaks assumptions we make in PlateCarree, where we try to essentially make it treat lat/lon as unchanged Cartesian coordinates; this is accomplished by undoing the scaling by the Earth's radius (and converting radians back to degrees).

This PR attempts to update PlateCarree to correctly function with Proj 9.8 by avoiding scaling hacks altogether and just use the latlong projection to accomplish what we intend with PlateCarree. In order to handle the offsets that were previously supported, we use the pm (Prime Meridian) PROJ parameter. (Using lon_0 in this case created problems with finding transformations in some cases because this ends up encoded as a remark.) pm causes some challenges in that it ends up attached to the datum, which violates some assumptions we were previously making.

This breaks some tests, but nothing that seems to be critical. One oddity is that some clipping around the edges of plots seem to be working less well for reasons I haven't identified.

This eliminates a warning from shapely deprecating the geos submodule.
Copy link
Copy Markdown
Contributor

@greglucas greglucas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this makes sense for Natural Earth at a minimum
https://www.naturalearthdata.com/features/

Projection: Natural Earth’s WGS84 projection is specified in shapefile’s PRJ file. It can also be described as a PROJ4 code [EPSG:4326](http://spatialreference.org/ref/epsg/wgs-84/) string “+proj=longlat +ellps=WGS84 +datum=WGS84 +no_defs”.

The thing that I've always wondered though is if we shouldn't just make a class LatLong() instead (or in addition to as an alternative spelling)?

Regarding your question about clipping being worse, do you know if this takes the geodesic interpolator path because pyproj thinks even the native "projection" definition is geodetic?

Implement the shift previously enabled with lon_0 using the prime
meridian (pm) argument.
@dopplershift
Copy link
Copy Markdown
Contributor Author

I agree we should also have a LatLong class to do this. If we do so, I would consider not having that class support lon_0, because that offset has created a lot of headaches trying to move from our old implementation to just using latlon. (See the dropping pm hackery I had to do here.) I would want to see what other ways we can use to implement the shifting people use lon_0 for with PlateCarree. In that event I would (probably with a quiet warning) deprecate PlateCarree and add a separate EquidistantCylindrical class. IMO, we've just been conflating too many things in PlateCarree for that to be salvageable API-wise.

No, it doesn't trigger Cartopy's geodesic interpolation (SphericalInterpolator), because while PROJ returns True for is_geographic, being a subclass of Projection means is_geodetic() for PlateCarree will always return False.

Test failure notes:

  • I need to dig into the tests failures more, especially for pcolormesh. Since we allow a sizable tolerance, I'm not sure what changes represent only a change against the (long-term) baseline versus what this PR changes against main.
  • A lot of the changes to the gridlining/labelling look like improvements.
  • test_gridliner_title_adjust shows some shifts of gridlines relative to the coastlines for the Mercator panel. I want to understand what's causing that better.
  • Not sure why test_cursor_values is getting different values

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants