raise exception on invalid authority for Datum, Ellipsoid, CoordinateOperation, PrimeMeridian

[snowman2]

#293

[jorisvandenbossche]

Question: you only update tests that use from_epsg, while the code change is in from_authority. I suppose that from_epsg uses from_authority under the hood, but that means we have no direct test for from_authority ?

[snowman2]

but that means we have no direct test for from_authority

I am unfamiliar with another authority other than EPSG, so I wasn't sure about how to test from_authority with another authority. However, it may be good to just test from_authority to ensure it does not change even if from_epsg does.

[snowman2]

from_authority tests added.

[jorisvandenbossche]

I am unfamiliar with another authority other than EPSG

Whenever I need some kind of crs of a specific type that I don't know about (while doing the reviews and testing here), I look at the new proj.db sources: https://github.com/OSGeo/proj.4/tree/master/data/sql (and eg this one has extra authority codes for IGNF: https://github.com/OSGeo/proj.4/blob/master/data/sql/ignf.sql)

(but testing it with EPSG as you did now is also perfectly fine of course!)

[jorisvandenbossche]

(but testing it with EPSG as you did now is also perfectly fine of course!)

Actually, it is not :-)

The other authorities don't necessarily have integer codes:

In [21]: pyproj.CRS.from_auth('IGNF', 'ETRS89UTM28')                                                                                                          
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-21-fac8b1789375> in <module>
----> 1 pyproj.CRS.from_auth('IGNF', 'ETRS89UTM28')

~/scipy/repos/pyproj/pyproj/crs.py in from_auth(cls, auth_name, code)
    243         ~CRS
    244         """
--> 245         if int(code) <= 0:
    246             raise CRSError("Authority codes are positive integers")
    247         return cls("{}:{}".format(auth_name, code))

ValueError: invalid literal for int() with base 10: 'ETRS89UTM28'

So we need to remove that restriction in from_auth, and move that to from_epsg

[snowman2]

So we need to remove that restriction in from_auth, and move that to from_epsg

Indeed we do. Good catch and thanks for digging into it 👍

[snowman2]

Okay, so I made a few changes to get it to work and pulled in a commit from #289 as it was needed as well (maybe #289 can be closed and just merge in this MR when ready), and I think we need a to_authority function for the CRS.__repr__ to better represent the IGNF authority.

>>> import pyproj
>>> cc = pyproj.CRS.from_authority('IGNF', 'ETRS89UTM28')
>>> cc2 = pyproj.CRS(cc.to_wkt())
>>> cc2
<Projected CRS: epsg:ETRS89UTM28>
Name: ETRS89 UTM Nord fuseau 28
Axis Info [cartesian]:
- X[east]: Easting (metre)
- Y[north]: Northing (metre)
Area of Use:
- name: EUROPE (ETRS89) - UTM NORD FUSEAU 28
- bounds: (-18.0, 27.5, -12.0, 71.5)
Coordinate Operation:
- name: UTM NORD FUSEAU 28
- method: Transverse Mercator
Datum: European Terrestrial Reference System 1989
- Ellipsoid: GRS 1980
- Prime Meridian: Greenwich
>>> cc = pyproj.CRS.from_authority('EPSG', 'ETRS89UTM28')  
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/snowal/scripts/pyproj/pyproj/crs.py", line 245, in from_auth
    return cls("{}:{}".format(auth_name, code))
  File "/home/snowal/scripts/pyproj/pyproj/crs.py", line 224, in __init__
    super(CRS, self).__init__(projstring)
  File "pyproj/_crs.pyx", line 1141, in pyproj._crs._CRS.__init__
    raise CRSError(
pyproj.exceptions.CRSError: Invalid projection: EPSG:ETRS89UTM28: (Internal Proj Error: proj_create: crs not found)

[snowman2]

Maybe in the repr, instead of to_epsg, should have a to_authority that iterates over proj_get_authorities_from_database until it finds a match - or not.

[snowman2]

Even better, have to_authority return both auth_name and auth_code and use NULL for auth name to query all auth names for the best match.
https://github.com/OSGeo/proj.4/blob/cd2f5b38046b7775feae08dfc8d8b1e59c1689f2/src/proj.h#L815

[snowman2]

Current iteration has the desired behavior now:

>>> import pyproj
>>> cc = pyproj.CRS.from_authority('IGNF', 'ETRS89UTM28')  
>>> cc
<Projected CRS: IGNF:ETRS89UTM28>
Name: ETRS89 UTM Nord fuseau 28
Axis Info [cartesian]:
- X[east]: Easting (metre)
- Y[north]: Northing (metre)
Area of Use:
- name: EUROPE (ETRS89) - UTM NORD FUSEAU 28
- bounds: (-18.0, 27.5, -12.0, 71.5)
Coordinate Operation:
- name: UTM NORD FUSEAU 28
- method: Transverse Mercator
Datum: European Terrestrial Reference System 1989
- Ellipsoid: GRS 1980
- Prime Meridian: Greenwich

>>> pyproj.CRS(cc.to_wkt())
<Projected CRS: IGNF:ETRS89UTM28>
Name: ETRS89 UTM Nord fuseau 28
Axis Info [cartesian]:
- X[east]: Easting (metre)
- Y[north]: Northing (metre)
Area of Use:
- name: EUROPE (ETRS89) - UTM NORD FUSEAU 28
- bounds: (-18.0, 27.5, -12.0, 71.5)
Coordinate Operation:
- name: UTM NORD FUSEAU 28
- method: Transverse Mercator
Datum: European Terrestrial Reference System 1989
- Ellipsoid: GRS 1980
- Prime Meridian: Greenwich

[jorisvandenbossche]

Some usage feedback:

• filtering by authority does not work:

In [11]: crs = pyproj.CRS.from_authority("IGNF", "ETRS89UTM28")                                                                                               

In [12]: crs.to_authority()                                                                                                                                   
Out[12]: ('IGNF', 'ETRS89UTM28')

In [13]: crs.to_authority('epsg')                                                                                                                             
Out[13]: ('IGNF', 'ETRS89UTM28')

[jorisvandenbossche]

Even better, have to_authority return both auth_name and auth_code and use NULL for auth name to query all auth names for the best match.

+1

[snowman2]

filtering by authority does not work:

I raised an issue about this in OSGeo/PROJ#1465

[snowman2]

Even fixed it on the master branch:

>>> from pyproj import CRS
>>> crs = CRS.from_authority("IGNF", "ETRS89UTM28") 
>>> crs.to_authority("EPSG")
('EPSG', '25828')

[snowman2]

Will have to wait for 6.1.0 release for tests to pass on this one.

[snowman2]

Looks like the to_authority fix will have to wait until the 6.1.1 release. I have a check based on the version in the tests, so it should be fine when updating to 6.1.1.