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

Disallow creation of non-CRS (eg pipeline) in CRS class #267

Closed
jorisvandenbossche opened this issue Apr 11, 2019 · 5 comments
Closed

Disallow creation of non-CRS (eg pipeline) in CRS class #267

jorisvandenbossche opened this issue Apr 11, 2019 · 5 comments

Comments

@jorisvandenbossche
Copy link
Contributor

From discussion with @snowman2

Currently, you can create a CRS object from eg a proj pipeline:

In [4]: pyproj.CRS("+proj=pipeline +ellps=GRS80 +step +proj=merc +step +proj=axisswap +order=2,1")                                                            
Out[4]: 
<CRS: +proj=pipeline +ellps=GRS80 +step +proj=merc +step ...>
Name: PROJ-based coordinate operation
Ellipsoid:
- UNDEFINED
Area of Use:
- UNDEFINED
Prime Meridian:
- UNDEFINED
Axis Info:
- UNDEFINED

which does not give you an actual CRS, but generic coordinate transformation. So it is probably a good idea to disallow this in the construction of a CRS object.

For this specific case we could check for proj=pipeline in the string. Or more in general we could check the proj_type and raise an error if it is not one of the CRS types (we could still do the first pipeline check as well to have a better error message for that specific case).

@snowman2
Copy link
Member

I think just need to check that proj_type == PJ_TYPE_CRS

@jorisvandenbossche
Copy link
Contributor Author

I think just need to check that proj_type == PJ_TYPE_CRS

Not sure if that is enough, as you have also PJ_TYPE_PROJECTED_CRS etc (don't know how such an equality check works, but I assume it is like an enum, and not aware that PJ_TYPE_PROJECTED_CRS is a subset of PJ_TYPE_CRS) ?

There is also proj_is_crs that can be probably used for this

@snowman2
Copy link
Member

The proj_is_crs looks perfect. I am glad you found that.

@snowman2
Copy link
Member

I have a less than optimal implementation in #263.

snowman2 added a commit to snowman2/pyproj that referenced this issue Apr 12, 2019
snowman2 added a commit to snowman2/pyproj that referenced this issue Apr 12, 2019
snowman2 added a commit to snowman2/pyproj that referenced this issue Apr 13, 2019
snowman2 added a commit that referenced this issue Apr 23, 2019
…s; update CRS repr) (#263)

* updated CRS repr for clarity; added BaseCRS for
datum/ellipsoid/prime_meridian

* add check to ensure input to CRS is a CRS (issue #267)

* added support for compound CRS (issue #265)

* updated repr to use EPSG code with 100% confidence for input & removed EPSG in repr

* added docstrings for cython classes; cleaned up unused attributes in crs cython classes; added examples for CRS usage

* fixed setting the coordinate operation name

* change boolean like objects from int to bool

* check if bound crs is geographic (issue #274)

* added support for from_auth to CRS,Ellipsoid,PrimeMeridian,CoordinateOperation,Datum 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
Projects
None yet
Development

No branches or pull requests

2 participants