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

Proj4 projection #461

Closed
wants to merge 4 commits into from
Closed

Conversation

om-henners
Copy link

Hi all,

This change modifies _EPSGProjection by making it a subclass of a new _Proj4Projection which takes a proj4 string and optionally projection bounds as parameters. Also adds a proj4 method to crs.py which calls the _Proj4Projection class directly so you can instantiate the projection given a proj4 string rather than just an EPSG code.

This has been ammended from the previous pull request to include unit tests and pull _Proj4Projection into crs.py instead of _epsg.py so that it doesn't have an implicit pyepsg requirement.

Any issues please give me a shout (and I'll email through the CLA tomorrow from work)

Modify _EPSGProjection by making it a subclass of a new _Proj4Projection which takes a proj4 string and optionally projection bounds as parameters. Also adds a proj4 method to crs.py which calls the _Proj4Projection class directly so we can instantiate the projection given a proj4 string rather than just an EPSG code.

Signed-off-by: Henry Walshaw <[email protected]>
Move _Proj4Projection out of _epsg so that it doesn't have an implicit requirement for pyepsg any more (and it probably didn't really belong there anyway)
Add a unit test for proj4 that's basically copying the unit test from epsg, without the requirement for pyepsg

Signed-off-by: Henry Walshaw <[email protected]>
Add myself to the contributors list
@pelson
Copy link
Member

pelson commented Aug 12, 2014

I'm not especially comfortable with this. When we implemented the EPSGProjection we limited the scope of the "proj4" awareness as much as possible as the long term goal is for cartopy to be able to convert from "proj4" strings to objects (and back again) without the need for having a catch-all class such as the Proj4Projection class you have implemented here.

I've actually got some prototype code which implements some of the conversion already, which essentially now needs to be re-written in a maintainable form. Once that is done, it would be a matter of entering a proj4 string into a function, and a suitable projection class pops out (or it would tell you if we haven't implemented sufficient keywords on the class just yet).

Clearly, anybody who is doing geospatial stuff is either passing around WKT or proj4 strings, so to play well with other tools cartopy really needs this kind of functionality (I'm looking forward to some examples using rasterio) so I'm tempted to accept this PR as a placeholder for the proj4 <-> cartopy class converter. Either way, the good news is that being just a (relatively simple) class means that even if it doesn't make it in, the behaviour can still be achieved on a per-user basis by implementing (or importing) the class from outside of cartopy.

Ok - that was a bit of an incoherent ramble, so please should if none of that makes any sense! 😄

Essentially, the point I'm trying to make is that I'm torn as to whether this is something that should be merged into cartopy or not. What do you think given this extra information @om-henners?

@om-henners
Copy link
Author

I'd certainly like to see WKT and Proj4 classes being implemented in cartopy. I origianlly wrote it because I did need to bring in some GIS data through rasterio and fiona (I'm happy to pass across an example if you'd like to see one).

The biggest issue I had with the way my proj4 method was implemented is that it only supports projected coordinate systems (in the same way that the epsg method currently does), so given the choice I'd rather see the full WKT and Proj4 projection classes implemented especially if you're already working on them.

In the meantime it would be nice to see my method merged in, but as you say it's pretty simple so it's not really necessary. I'm happy to put it up in a Gist and let people use it as they need to. I suspect the Gist will be better solution in the long run anyway; people that import the method themselves won't have to go back and re-write code on upgrading cartopy with the updated projection classes included.

@LukeC92
Copy link

LukeC92 commented Oct 6, 2017

Is cartopy now able to convert proj4 and WKT strings to and from objects? If not would there be any benefit to adding something like @om-henners method to the modern cartopy? Or does this pull request need to be closed?

@pelson pelson added Component: proj Experience-needed: high Type: Enhancement Blocked: CLA needed See https://scitools.org.uk. Submit the form at: https://scitools.org.uk/cla/v4/form labels Nov 20, 2017
@pelson
Copy link
Member

pelson commented Nov 21, 2017

Is cartopy now able to convert proj4 and WKT strings to and from objects?

No, unfortunately not. #813 relates.

I'm not in favour of a Proj4Projection class, as it doesn't encapsulate the projection appropriately. The solution that I'd like to see is a cartopy <> proj4 transformation function.

Closing this as a won't fix.

@pelson pelson closed this Nov 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocked: CLA needed See https://scitools.org.uk. Submit the form at: https://scitools.org.uk/cla/v4/form Component: proj Experience-needed: high Type: Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants