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

API: CRS.datum being a CRS itself ? #262

Closed
jorisvandenbossche opened this issue Apr 10, 2019 · 7 comments
Closed

API: CRS.datum being a CRS itself ? #262

jorisvandenbossche opened this issue Apr 10, 2019 · 7 comments
Labels
enhancement proposal Idea for a new feature.

Comments

@jorisvandenbossche
Copy link
Contributor

(this might be more a question on why this was implemented like this, but since I don't know of a better place to ask, opening an issue here. If there is a better venue to ask such questions, let me know!)

So if you define a CRS, eg Web Mercator here, and get the datum property, you get back a CRS object:

In [7]: crs = pyproj.CRS.from_epsg(3857)

In [8]: crs  
Out[8]: 
<CRS: epsg:3857>
Name: WGS 84 / Pseudo-Mercator
Ellipsoid:
- semi_major_metre: 6378137.00
- semi_minor_metre: 6356752.31
- inverse_flattening: 298.26
Area of Use:
- name: World - 85°S to 85°N
- bounds: (-180.0, -85.06, 180.0, 85.06)
Prime Meridian:
- longitude: 0.0000
- unit_name: degree
- unit_conversion_factor: 0.01745329
Axis Info:
- Easting[X] (east) EPSG:9001 (metre)
- Northing[Y] (north) EPSG:9001 (metre)

In [9]: crs.datum 
Out[9]: 
<CRS: DATUM["World Geodetic System 1984", ELLIPSOID["WGS ...>
Name: World Geodetic System 1984
Ellipsoid:
- semi_major_metre: 6378137.00
- semi_minor_metre: 6356752.31
- inverse_flattening: 298.26
Area of Use:
- UNDEFINED
Prime Meridian:
- longitude: 0.0000
- unit_name: degree
- unit_conversion_factor: 0.01745329
Axis Info:
- UNDEFINED

but, since this is a Datum, it is not a complete CRS object. For example, it has no AxisInfo (by definition I think, as only a CoordinateSystem has that), it also has no proj4 string representation (crs.datum.to_proj4() returns None), etc

I know that from the proj4 side, both CRS and Datum are PJ objects. So it can make sense to have them in a single class in the python wrapper. But, we already don't allow all types of PJ objects in the pyproj.CRS class (eg an Ellipsoid is also a PJ object and can have a EPSG number, but for Ellipsoid we have a separate class).
And moreover, I think from a user perspective, it might be useful and clearer that a CRS class always is a CRS (and not Datum, Ellipsoid, ..)

@snowman2
Copy link
Member

You bring up a good question. It was done this way as it is a PJ* object and some of the methods worked with the already existing CRS class and I didn't want to limit what the user could do without some research first. I think with some research a better defined Datum class could be created that makes things more clear from a users perspective.

@snowman2 snowman2 added enhancement proposal Idea for a new feature. labels Apr 10, 2019
@snowman2
Copy link
Member

If there is a better venue to ask such questions, let me know!

This is a great place for the questions you are asking. Maybe in the future a gitter thread can be created for a forum for pyproj users.

@snowman2
Copy link
Member

Also, maybe adding access to:
proj_crs_get_horizontal_datum
as well as:
proj_crs_get_datum
would be useful.

@jorisvandenbossche
Copy link
Contributor Author

jorisvandenbossche commented Apr 10, 2019

It's maybe a bit broader discussion than just Datum. Also the CoordinateSystem and CoordinateOperation might deserve its class (there is also a proj_crs_get_coordinate_system and proj_crs_get_coordoperation). While for Ellipsoid and PrimeMeridian, we have classes, but with less functionality (eg no conversion to wkt, although that is maybe the only thing)

@snowman2
Copy link
Member

I think each one having a class with targeted functionality is a good idea.

@jorisvandenbossche
Copy link
Contributor Author

I think the general question is to what extent we want to model the different types in the C++ ISO-19111 API.

One could imagine a class hierarchy as:

class _Base:
    cdef PJ * projobj
    cdef PJ_CONTEXT * projctx
    
    def name
    def to_wkt(self, version, multiline)    


class Ellipsoid(_Base):
    
    def semi_major_metre
    def semi_minor_metre
    def inverse_flattening
    ..
    
class PrimeMeridian(_Base):
    
    def longitude
    def unit_name
    def unit_conversion_factor
    ..
    
class Datum(_Base)
    def ellipsoid -> Ellipsoid
    def prime_meridian -> PrimeMeridian


class CoordinateSystem(_Base):
    def type = proj_cs_get_type   # cartesian, ellipsoidal, .. 
    def axis_list -> [list of AxisInfo objects]
    

class CoordinateOperation(_Base):
    def method -> from proj_coordoperation_get_method_info
    parameters ... (maybe a dict?)
    
    def to_proj4(self)
    

class CRS(_Base):

    def datum -> Datum or None
    def ellipsoid -> self.datum.ellipsoid or proj_get_ellipsoid
    def prime_meridian -> self.datum.prime_meridian or proj_get_prime_meridian
    def coordinate_operation -> CoordinateOperation or None (eg exists if projected CRS)
    def coordinate_system -> CoordinateSystem
    
    def area_of_use
    
    def from_...
    def to_...

(that seems a lot, but we already have the majority of them)

Of course, one could also go a step further, and define a bunch of subclasses for each of them to completely follow the ISO-19111, but that might be too much.
Or on the other hand, we could have a single class wrapping a PJ object that exposes all this functionality (like now CRS and Datum were a single class, although we are changing that). But from a user perspective, I think the different classes are nicer.

As an external user (in my case from GeoPandas), I mainly care about a good user-friendly CRS class (and maybe a CoordinateOperionan class, although not sure how that would relate to the existing Proj / Transformer classes).

@snowman2
Copy link
Member

I am going to close this as the solution is merged into master. Thanks for bringing up the idea!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement proposal Idea for a new feature.
Projects
None yet
Development

No branches or pull requests

2 participants