FIX: Handle non-earth bodies within Projections #2283
Conversation
There were several places that initialized PlateCarree with no globe. We want to use the same globe as the Projection we are transforming between if the user doesn't pass one in explicitly.
lib/cartopy/mpl/ticker.py
Outdated
| if source.globe != self._target_projection.globe: | ||
| # The transforms need to use the same globe | ||
| self._target_projection = ccrs.PlateCarree(globe=source.globe) |
There was a problem hiding this comment.
This is the only part that gives me pause. I'm not really sure what's going on here.
There was a problem hiding this comment.
That is because it is initialized as a class variable above without a globe:
cartopy/lib/cartopy/mpl/ticker.py
Line 23 in 0d58449
then we try to do the transform here in the call and it raises if the globes are different. But, we can't know what the source projection is until this point, and we don't want to create a new projection every call either I don't think, so I added this check for that case.
There was a problem hiding this comment.
Could we implement set_axis (which exists from Formatter->TickHelper) to set what we need at that time?
There was a problem hiding this comment.
That is a good idea. I did that in a separate commit so it is easier to see the update. All of the tests we have currently call .axis = Mock(...), which avoids the set_axis() path. So, do we think that anyone would call .axis = ... on the formatter and thus we should keep the logic in the call method instead... Or I guess we could turn .axis into a property?
There was a problem hiding this comment.
Given that .axis and set_axis() are upstream APIs, I think it makes the most sense for any notion of axis being a property to be handled there.
I feel comfortable not worrying about users manually setting .axis; my advice in that case would be for users to use set_axis too.
Move the source and target projection calculation into the set_axis() call instead of the __call__ method.
There were several places that initialized PlateCarree with no globe. We want to use the same globe as the Projection we are transforming to/from if the user doesn't pass one in explicitly.
The OSGB test image had the gridlines shifted ever so slightly because that projection uses a different globe than the default PlateCarree (but still Earth radii, so no failures from PROJ). I think that we want to gridline/label to default to the same globe and this was incorrect before, but I'm also not 100% positive about that, so it'd be good for someone else to verify that logic. To avoid updating the test image I forced a
crs=PlateCarree()with a default globe to be used within the gridliner to get the previous behavior.closes #2007