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

Add support for CompoundCRS #1227

Merged
merged 2 commits into from
Dec 4, 2018
Merged

Conversation

rhattersley
Copy link
Member

Rationale

Allow Cartopy to make use of EPSG codes that define a compound CRS.

See rhattersley/pyepsg#10.

Implications

A projection can now be created using an EPSG code for a compound CRS, whereas previously it would have raised an error.

@stickler-ci
Copy link

I couldn't find a .stickler.yml file in this repository. I can make one for you, or you can create one by following the documentation.

@rhattersley
Copy link
Member Author

Do I need to do something to fix the appveyor problem? Or is #1224 tackling this?

@@ -41,7 +41,9 @@ class _EPSGProjection(ccrs.Projection):
def __init__(self, code):
import pyepsg
projection = pyepsg.get(code)
if not isinstance(projection, pyepsg.ProjectedCRS):
if not (isinstance(projection, pyepsg.ProjectedCRS) or
(hasattr(pyepsg, 'CompoundCRS') and
Copy link
Member

Choose a reason for hiding this comment

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

I'd be extremely tempted to update pyepsg dependency to >=0.4. Relevant locations : https://github.com/SciTools/cartopy/blob/master/requirements/epsg.txt and https://github.com/SciTools/cartopy/blob/master/INSTALL

Copy link
Member Author

Choose a reason for hiding this comment

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

Bump the dependency and simplify this logic? Happy to do so if that's what you prefer.

Copy link
Member

@pelson pelson Dec 4, 2018

Choose a reason for hiding this comment

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

Let's go for it (bump and simplify).

Copy link
Member

@pelson pelson left a comment

Choose a reason for hiding this comment

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

No need to worry about appveyor yet (it is a shame it is failing - needs reconfiguring).
This is essentially good to go. If you have an inclination to bump the minimum version dep, do it. Otherwise, I'd merge anyway. 👍

@rhattersley
Copy link
Member Author

Should be good to go now. 👍

@pelson pelson merged commit bbfcdfb into SciTools:master Dec 4, 2018
@pelson
Copy link
Member

pelson commented Dec 4, 2018

Great, thanks @rhattersley. And welcome back to cartopy contributions 👍 😉

@rhattersley rhattersley deleted the pyepsg-compound-crs branch December 5, 2018 11:39
@rhattersley
Copy link
Member Author

Nice to be back 😉

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

Successfully merging this pull request may close these issues.

3 participants