ENH: Add CRS PROJ JSON support #2212

snowman2 · 2021-06-18T01:10:25Z

sgillies · 2021-06-18T16:29:59Z

As always, the code is impeccable! Thank you, @snowman2.

We could make this feature more idiomatically "Pythonic" by shoehorning it into the existing CRS.from_dict and CRS.to_dict methods with the help of a proj_json=True option. Existing methods of Rasterio, fiona, and shapely all take or return dicts or Python mappings instead of JSON and I'd like to preserve this design.

snowman2 · 2021-06-18T17:03:58Z

@sgillies, that is a good idea. The current design is to match how it was done with pyproj for consistency. Too bad we didn't have this conversation sooner 😄 .

We could make this feature more idiomatically "Pythonic" by shoehorning it into the existing CRS.from_dict and CRS.to_dict methods with the help of a proj_json=True option.

I am understanding this to mean that we replace to_json/from_json/to_json_dict/from_json_dict with logic inside of from_dict/to_dict, correct?

A CRS.to_dict(proj_json=True) option sounds resonable.
I think it would be pretty simple to have CRS.from_dict support PROJ JSON dictionaries as that logic is within the from_string/from_user_input methods at the moment and additional kwargs wouldn't be needed. This will make the CRS constructor also work with PROJ JSON dicts.

Existing methods of Rasterio, fiona, and shapely all take or return dicts or Python mappings instead of JSON and I'd like to preserve this design.

Ideally, I would like to see the PROJ dict replaced by the PROJ JSON dict as the default for the rasterio CRS class as it better represents the CRS and is less lossy. I think making from_dict support PROJ JSON is the step in the right direction for that.

sgillies · 2021-06-18T18:32:07Z

I am understanding this to mean that we replace to_json/from_json/to_json_dict/from_json_dict with logic inside of from_dict/to_dict, correct?

Correct. Meaning that in CRS.to_dict we deserialize the PROJ JSON and return a dict and in CRS.from_dict we serialize a dict and pass JSON to GDAL's OSR method. There's a little bit of waste, but unless it's significantly slower, we should accept it.

snowman2 · 2021-06-19T00:59:12Z

I made the suggested changes above, however I did keep in to_json for a couple of reasons:

We need it anyway for to_dict()
Makes testing input PROJ JSON strings more convenient.
I like the pretty formatted version of PROJ JSON better than pprint:

>>> from rasterio.crs import CRS
>>> cc = CRS.from_epsg(4326)
>>> from pprint import pprint
>>> pprint(cc.to_dict(proj_json=True))
{'$schema': 'https://proj.org/schemas/v0.2/projjson.schema.json',
 'area': 'World.',
 'bbox': {'east_longitude': 180,
          'north_latitude': 90,
          'south_latitude': -90,
          'west_longitude': -180},
 'coordinate_system': {'axis': [{'abbreviation': 'Lat',
                                 'direction': 'north',
                                 'name': 'Geodetic latitude',
                                 'unit': 'degree'},
                                {'abbreviation': 'Lon',
                                 'direction': 'east',
                                 'name': 'Geodetic longitude',
                                 'unit': 'degree'}],
                       'subtype': 'ellipsoidal'},
 'datum_ensemble': {'accuracy': '2.0',
                    'ellipsoid': {'inverse_flattening': 298.257223563,
                                  'name': 'WGS 84',
                                  'semi_major_axis': 6378137},
                    'id': {'authority': 'EPSG', 'code': 6326},
                    'members': [{'id': {'authority': 'EPSG', 'code': 1166},
                                 'name': 'World Geodetic System 1984 '
                                         '(Transit)'},
                                {'id': {'authority': 'EPSG', 'code': 1152},
                                 'name': 'World Geodetic System 1984 (G730)'},
                                {'id': {'authority': 'EPSG', 'code': 1153},
                                 'name': 'World Geodetic System 1984 (G873)'},
                                {'id': {'authority': 'EPSG', 'code': 1154},
                                 'name': 'World Geodetic System 1984 (G1150)'},
                                {'id': {'authority': 'EPSG', 'code': 1155},
                                 'name': 'World Geodetic System 1984 (G1674)'},
                                {'id': {'authority': 'EPSG', 'code': 1156},
                                 'name': 'World Geodetic System 1984 (G1762)'}],
                    'name': 'World Geodetic System 1984 ensemble'},
 'id': {'authority': 'EPSG', 'code': 4326},
 'name': 'WGS 84',
 'scope': 'Horizontal component of 3D system.',
 'type': 'GeographicCRS'}
>>> print(cc.to_json(pretty=True))
{
  "$schema": "https://proj.org/schemas/v0.2/projjson.schema.json",
  "type": "GeographicCRS",
  "name": "WGS 84",
  "datum_ensemble": {
    "name": "World Geodetic System 1984 ensemble",
    "members": [
      {
        "name": "World Geodetic System 1984 (Transit)",
        "id": {
          "authority": "EPSG",
          "code": 1166
        }
      },
      {
        "name": "World Geodetic System 1984 (G730)",
        "id": {
          "authority": "EPSG",
          "code": 1152
        }
      },
      {
        "name": "World Geodetic System 1984 (G873)",
        "id": {
          "authority": "EPSG",
          "code": 1153
        }
      },
      {
        "name": "World Geodetic System 1984 (G1150)",
        "id": {
          "authority": "EPSG",
          "code": 1154
        }
      },
      {
        "name": "World Geodetic System 1984 (G1674)",
        "id": {
          "authority": "EPSG",
          "code": 1155
        }
      },
      {
        "name": "World Geodetic System 1984 (G1762)",
        "id": {
          "authority": "EPSG",
          "code": 1156
        }
      }
    ],
    "ellipsoid": {
      "name": "WGS 84",
      "semi_major_axis": 6378137,
      "inverse_flattening": 298.257223563
    },
    "accuracy": "2.0",
    "id": {
      "authority": "EPSG",
      "code": 6326
    }
  },
  "coordinate_system": {
    "subtype": "ellipsoidal",
    "axis": [
      {
        "name": "Geodetic latitude",
        "abbreviation": "Lat",
        "direction": "north",
        "unit": "degree"
      },
      {
        "name": "Geodetic longitude",
        "abbreviation": "Lon",
        "direction": "east",
        "unit": "degree"
      }
    ]
  },
  "scope": "Horizontal component of 3D system.",
  "area": "World.",
  "bbox": {
    "south_latitude": -90,
    "west_longitude": -180,
    "north_latitude": 90,
    "east_longitude": 180
  },
  "id": {
    "authority": "EPSG",
    "code": 4326
  }
}

sgillies · 2021-07-26T20:13:57Z

@snowman2 I'm sorry, but I can't get behind a to_json method that's redundant with json.dump(crs.to_dict). It also goes against rasterio's style to have a special formatting of JSON only for projections.

snowman2 · 2021-07-27T00:15:52Z

Sounds good. Does having to_json on the Cython class only sound like a reasonable method to address this?

sgillies · 2021-10-14T21:23:27Z

I'm giving this another review. Let's do it for 1.3a2.

sgillies · 2021-10-14T21:32:03Z

rasterio/_crs.pyx

    @staticmethod
    def from_dict(initialdata=None, **kwargs):
-        """Make a CRS from a PROJ dict
+        """Make a CRS from a dict of PROJ parameters or PROJ JSON


Suggested change

"""Make a CRS from a dict of PROJ parameters or PROJ JSON

"""Make a CRS from a PROJ4 or PROJJSON style dict.

PROJJSON text is not accepted.

sgillies · 2021-10-14T21:33:06Z

rasterio/crs.py

-    def to_dict(self):
-        """Convert CRS to a PROJ4 dict
+    def to_dict(self, proj_json=False):
+        """Convert CRS to a PROJ dict


Suggested change

"""Convert CRS to a PROJ dict

"""Convert CRS to a PROJ4 or PROJJSON style dict.

sgillies · 2021-10-14T21:37:54Z

rasterio/crs.py

+            except CRSError:
+                return {}
+
+    def to_json(self, pretty: bool=False, indentation: int=2) -> str:


Let's remove this method. Python's json module is perfectly adequate and json.dumps(crs.to_dict(proj_json=True)) is easy to write.

sgillies · 2021-10-14T21:40:19Z

rasterio/crs.py

            raise CRSError("Undefined CRS has no dict representation")

+        if proj_json:
+            proj_json = self.to_json()


Suggested change

proj_json = self.to_json()

proj_json = self._crs.to_json()

sgillies

@snowman2 A couple of changes requested inline. Look forward to having this in 1.3a2!

snowman2 · 2021-10-15T13:19:28Z

Hi @sgillies, thanks for the review 👍. When do you need this updated by? (If you need changes soon, feel free to make any changes you would like to this PR).

sgillies · 2021-10-15T16:51:22Z

@snowman2 I'd like to do 1.3a2 early next week. I'd be happy to make the changes myself if that doesn't fit your schedule.

snowman2 · 2021-10-15T19:23:15Z

I am uncertain whether I would be able to meet that timeline. I think it would be safer for you to finish this off to ensure it is done in time. Thanks @sgillies 👍

sgillies · 2021-10-19T22:19:23Z

Superseded by #2317.

snowman2 · 2021-10-20T00:18:03Z

Thanks @sgillies 👍

snowman2 force-pushed the projjson branch from d7ddb33 to bc3b1c5 Compare June 18, 2021 01:23

snowman2 marked this pull request as ready for review June 18, 2021 01:23

snowman2 force-pushed the projjson branch 2 times, most recently from e393b1a to f56ef8d Compare June 18, 2021 02:28

ENH: Add CRS PROJ JSON support

0ff7682

snowman2 force-pushed the projjson branch from f56ef8d to d68ee72 Compare June 19, 2021 00:53

REF: Add PROJ JSON support to_dict/from_dict

a8f7e11

snowman2 force-pushed the projjson branch from d68ee72 to a8f7e11 Compare June 19, 2021 01:05

sgillies reviewed Oct 14, 2021

View reviewed changes

sgillies requested changes Oct 14, 2021

View reviewed changes

sgillies added this to the 1.3.0 milestone Oct 14, 2021

sgillies mentioned this pull request Oct 19, 2021

PROJJSON #2317

Merged

sgillies closed this Oct 19, 2021

snowman2 deleted the projjson branch October 20, 2021 00:11

-        """Make a CRS from a dict of PROJ parameters or PROJ JSON
+        """Make a CRS from a PROJ4 or PROJJSON style dict.
+        PROJJSON text is not accepted.

	"""Convert CRS to a PROJ dict
	"""Convert CRS to a PROJ4 or PROJJSON style dict.

ENH: Add CRS PROJ JSON support #2212

ENH: Add CRS PROJ JSON support #2212

Uh oh!

Conversation

snowman2 commented Jun 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sgillies commented Jun 18, 2021

Uh oh!

snowman2 commented Jun 18, 2021

Uh oh!

sgillies commented Jun 18, 2021

Uh oh!

snowman2 commented Jun 19, 2021

Uh oh!

sgillies commented Jul 26, 2021

Uh oh!

snowman2 commented Jul 27, 2021

Uh oh!

sgillies commented Oct 14, 2021

Uh oh!

sgillies Oct 14, 2021

Choose a reason for hiding this comment

Uh oh!

sgillies Oct 14, 2021

Choose a reason for hiding this comment

Uh oh!

sgillies Oct 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sgillies Oct 14, 2021

Choose a reason for hiding this comment

Uh oh!

sgillies left a comment

Choose a reason for hiding this comment

Uh oh!

snowman2 commented Oct 15, 2021

Uh oh!

sgillies commented Oct 15, 2021

Uh oh!

snowman2 commented Oct 15, 2021

Uh oh!

sgillies commented Oct 19, 2021

Uh oh!

snowman2 commented Oct 20, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

snowman2 commented Jun 18, 2021 •

edited

Loading

sgillies Oct 14, 2021 •

edited

Loading