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

pyproj.CRS.from_cf deviates from CF standard guidelines #401

Closed
benjwadams opened this issue Aug 19, 2019 · 6 comments
Closed

pyproj.CRS.from_cf deviates from CF standard guidelines #401

benjwadams opened this issue Aug 19, 2019 · 6 comments
Labels

Comments

@benjwadams
Copy link

benjwadams commented Aug 19, 2019

Code Sample, a copy-pastable example if possible

crs = pyproj.CRS.from_cf({'grid_mapping_name': "lambert_conformal_conic", 'standard_parallel': 25.0, 'longitude_of_central_meridian': 265.0, 'latitude_of_projection_origin': 25.0,
    'crs_wkt': """PROJCS["North_America_Lambert_Conformal_Conic",
       GEOGCS["GCS_North_American_1983",
           DATUM["North_American_Datum_1983",
               SPHEROID["GRS_1980",6378137,298.257222101]],
            PRIMEM["Greenwich",0],
            UNIT["Degree",0.017453292519943295]],
       PROJECTION["Lambert_Conformal_Conic_2SP"],
        PARAMETER["False_Easting",0],
         PARAMETER["False_Northing",0],
         PARAMETER["Central_Meridian",-96],
         PARAMETER["Standard_Parallel_1",20],
         PARAMETER["Standard_Parallel_2",60],
         PARAMETER["Latitude_Of_Origin",40],
         UNIT["Meter",1],
        AUTHORITY["EPSG","102009"]]"""})

print(crs.to_string())

PROJCS["North_America_Lambert_Conformal_Conic",
GEOGCS["GCS_North_American_1983",
DATUM["North_American_Datum_1983",
SPHEROID["GRS_1980",6378137,298.257222101]],
PRIMEM["Greenwich",0],
UNIT["Degree",0.017453292519943295]],
PROJECTION["Lambert_Conformal_Conic_2SP"],
PARAMETER["False_Easting",0],
PARAMETER["False_Northing",0],
PARAMETER["Central_Meridian",-96],
PARAMETER["Standard_Parallel_1",20],
PARAMETER["Standard_Parallel_2",60],
PARAMETER["Latitude_Of_Origin",40],
UNIT["Meter",1],
AUTHORITY["EPSG","102009"]]

Problem description

http://cfconventions.org/cf-conventions/cf-conventions.html#use-of-the-crs-well-known-text-format mentions

There will be occasions when a given CRS property value is duplicated in both a single-property grid mapping attribute and the crs_wkt attribute. In such cases the onus is on data producers to ensure that the property values are consistent. However, in those situations where two values of a given property are different, then the value specified by the single-property attribute shall take precedence. For example, if the semi-major axis length of the ellipsoid is defined by the grid mapping attribute semi_major_axis and also by the crs_wkt attribute (via the WKT SPHEROID[…​] element) then the former, being the more specific attribute, takes precedence. Naturally if the two values are equal then no ambiguity arises.

pyproj.CRS.from_cf instead looks for the crs_wkt attribute and if it is found, only returns the CRS generated from that.

pyproj/pyproj/crs.py

Lines 602 to 627 in ca68a21

@staticmethod
def from_cf(in_cf, errcheck=False):
"""
This converts a Climate and Forecast (CF) Grid Mapping Version 1.8
dict to a :obj:`~pyproj.crs.CRS` object.
.. warning:: Parameters may be lost if a mapping
from the CF parameter is not found. For best results
store the WKT of the projection in the crs_wkt attribute.
Parameters
----------
in_cf: dict
CF version of the projection.
errcheck: bool, optional
If True, will warn when parameters are ignored. Defaults to False.
Returns
-------
CRS
"""
in_cf = in_cf.copy() # preserve user input
if "crs_wkt" in in_cf:
return CRS(in_cf["crs_wkt"])
elif "spatial_ref" in in_cf: # for previous supported WKT key
return CRS(in_cf["spatial_ref"])

This contradicts the CF standard, which states that the individual, more specific parameters should override the crs_wkt values if there are differences between the two.

Expected Output

Environment Information

  • Output from: python -c "import pyproj; pyproj.show_versions()"
System:
    python: 3.6.5 (default, Apr 10 2018, 17:08:37)  [GCC 4.8.5 20150623 (Red Hat 4.8.5-16)]
executable: /home/badams/.virtualenvs/cchecker_py3.6/bin/python
   machine: Linux-3.10.0-862.14.4.el7.x86_64-x86_64-with-centos-7.5.1804-Core

PROJ:
      PROJ: 6.1.0
  data dir: /home/badams/.virtualenvs/cchecker_py3.6/lib/python3.6/site-packages/pyproj/proj_dir/share/proj

Python deps:
    pyproj: 2.2.1
       pip: 19.2.2
setuptools: 41.1.0
    Cython: 0.29.12
     aenum: None

Installation method

  • conda, pip wheel, from source, etc...

pip

@benjwadams benjwadams added the bug label Aug 19, 2019
@snowman2
Copy link
Member

Thanks for the report, we will definitely take this into consideration on future iterations.

Current design reasoning:

  1. WKT strings are better than PROJ strings for fully representing a CRS ref.
  2. To use the other parameters, we would have to drop the WKT representation and represent the CRS using PROJ parmeters. This could be lossy and not desirable.

Potential solution:

Use PROJ JSON to modify the CRS with the specified parameters PROJ JSON PR. This will likely need to be a 2.4.0+ update and will need to use PROJ 6.2.0 or the release that adds PROJ JSON.

@benjwadams
Copy link
Author

I think it might be worthwhile to issue warnings in future versions. Even though it's the behavior specified by CF, it might be good to notify the user that the CF grid mapping parameters are overriding what's been provided in the CRS WKT.

@snowman2
Copy link
Member

snowman2 commented Mar 1, 2020

@benjwadams, I attempted to make the CRS better represented by using the WKT form of the projection. However, if the CF version of the coordinate system is not properly able to be supported within pyproj itself then the WKT form will be more accurate. As things currently are, pyproj won't follow the convention mentioned in this issue as it would be unsafe to do so (see #536) .

That being said, I think adding a documentation page about how to be fully CF compliant in respect to this issue would be a good addition and will resolve this issue as sensibly as possible (i.e. how to build the coordinate system yourself and compare the CRS generated from the CF parameters versus the CRS generated from the WKT).

@snowman2
Copy link
Member

Just had a CF breakout session & we all agreed upon the wording change: cf-convention/cf-conventions#222 (comment)

If accepted, that means that the current behavior of pyproj is acceptable.

@snowman2
Copy link
Member

Closing with this comment: cf-convention/cf-conventions#222 (comment)

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

No branches or pull requests

2 participants