From bb798f57f3f660c5ef886ed5d6a8b3279a694520 Mon Sep 17 00:00:00 2001 From: Johan Schreurs Date: Thu, 17 Aug 2023 21:54:52 +0200 Subject: [PATCH 1/4] Issue #437 Better documentation of CRS code usage, and slightly more test coverage --- docs/api.rst | 2 +- openeo/rest/datacube.py | 20 ++++++++++++++++++-- openeo/util.py | 23 ++++++++++++++++++++++- tests/test_util.py | 28 ++++++++++++++++++++++++++++ 4 files changed, 69 insertions(+), 4 deletions(-) diff --git a/docs/api.rst b/docs/api.rst index 097633709..5085aa2ef 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -126,7 +126,7 @@ openeo.util ------------- .. automodule:: openeo.util - :members: to_bbox_dict, BBoxDict, load_json_resource + :members: to_bbox_dict, BBoxDict, load_json_resource, normalize_crs openeo.processes diff --git a/openeo/rest/datacube.py b/openeo/rest/datacube.py index 1cff5d08e..83bba992f 100644 --- a/openeo/rest/datacube.py +++ b/openeo/rest/datacube.py @@ -285,7 +285,7 @@ def filter_bbox( >>> cube.filter_bbox(west=3, south=51, east=4, north=52, crs=4326) - With a (west, south, east, north) list or tuple - (note that EPSG:4326 is the default CRS, so it's not nececarry to specify it explicitly):: + (note that EPSG:4326 is the default CRS, so it's not necessary to specify it explicitly):: >>> cube.filter_bbox([3, 51, 4, 52]) >>> cube.filter_bbox(bbox=[3, 51, 4, 52]) @@ -308,7 +308,7 @@ def filter_bbox( >>> cube.filter_bbox(bbox_param) >>> cube.filter_bbox(bbox=bbox_param) - - With a CRS other than EPSG 4326:: + - With a CRS other than EPSG 4326 (**see also parameters: crs**):: >>> cube.filter_bbox(west=652000, east=672000, north=5161000, south=5181000, crs=32632) @@ -318,6 +318,17 @@ def filter_bbox( >>> west, east, north, south = 3, 4, 52, 51 >>> cube.filter_bbox(west, east, north, south) + :param crs: data structure that encodes a CRS, typically just an int or string value. + If the ``pyproj`` library is available, everything supported by it is allowed. + + Integers always refer to the EPSG code that has that number assigned. + A string that contains only an integer it is treated the same way. + + You can also specify EPSG codes as a formatted string, "EPSG:number". + For example, the following string and int values all specify to the same CRS: + ``"EPSG:4326"``, ``"4326"``, and the integer ``4326``. + + .. seealso:: `openeo.org documentation on openeo.util.normalize_crs `_ for the most up to date information. """ if args and any(k is not None for k in (west, south, east, north, bbox)): raise ValueError("Don't mix positional arguments with keyword arguments.") @@ -810,6 +821,11 @@ def _get_geometry_argument( ) -> Union[dict, Parameter, PGNode]: """ Convert input to a geometry as "geojson" subtype object. + + :param crs: data structure that encodes a CRS, typically just an int or string value. + If the ``pyproj`` library is available, everything supported by it is allowed. + .. seealso:: `openeo.org documentation on openeo.util.normalize_crs `_ for the most up to date information. + """ if isinstance(geometry, (str, pathlib.Path)): # Assumption: `geometry` is path to polygon is a path to vector file at backend. diff --git a/openeo/util.py b/openeo/util.py index bd9d8814a..067206423 100644 --- a/openeo/util.py +++ b/openeo/util.py @@ -536,6 +536,19 @@ class BBoxDict(dict): Dictionary based helper to easily create/work with bounding box dictionaries (having keys "west", "south", "east", "north", and optionally "crs"). + crs: + data structure that encodes a CRS, typically just an int or string value. + If the ``pyproj`` library is available, everything supported by it is allowed. + + Integers always refer to the EPSG code that has that number assigned. + A string that contains only an integer it is treated the same way. + + You can also specify EPSG codes as a formatted string, "EPSG:number". + For example, the following string and int values all specify to the same CRS: + ``"EPSG:4326"``, ``"4326"``, and the integer ``4326``. + + .. seealso:: `openeo.org documentation on openeo.util.normalize_crs `_ for the most up to date information. + .. versionadded:: 0.10.1 """ @@ -657,7 +670,15 @@ def normalize_crs(crs: Any, *, use_pyproj: bool = True) -> Union[None, int, str] Other data structures will not be accepted. :param crs: data structure that encodes a CRS, typically just an int or string value. - If the ``pyproj`` library is available, everything supported by it is allowed + If the ``pyproj`` library is available, everything supported by it is allowed. + + Integers always refer to the EPSG code that has that number assigned. + A string that contains only an integer it is treated the same way. + + You can also specify EPSG codes as a formatted string, "EPSG:number". + For example, the following string and int values all specify to the same CRS: + ``"EPSG:4326"``, ``"4326"``, and the integer ``4326``. + :param use_pyproj: whether ``pyproj`` should be leveraged at all (mainly useful for testing the "no pyproj available" code path) diff --git a/tests/test_util.py b/tests/test_util.py index a823c6ecc..e754797be 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -701,6 +701,13 @@ def test_init(self): "north": 4, "crs": 4326, } + assert BBoxDict(west=1, south=2, east=3, north=4, crs="4326") == { + "west": 1, + "south": 2, + "east": 3, + "north": 4, + "crs": 4326, + } def test_repr(self): d = BBoxDict(west=1, south=2, east=3, north=4) @@ -727,6 +734,13 @@ def test_to_bbox_dict_from_sequence(self): "north": 4, "crs": 4326, } + assert to_bbox_dict([1, 2, 3, 4], crs="4326") == { + "west": 1, + "south": 2, + "east": 3, + "north": 4, + "crs": 4326, + } def test_to_bbox_dict_from_sequence_mismatch(self): with pytest.raises(InvalidBBoxException, match="Expected sequence with 4 items, but got 3."): @@ -752,6 +766,13 @@ def test_to_bbox_dict_from_dict(self): "north": 4, "crs": 4326, } + assert to_bbox_dict({"west": 1, "south": 2, "east": 3, "north": 4, "crs": "4326"}) == { + "west": 1, + "south": 2, + "east": 3, + "north": 4, + "crs": 4326, + } assert to_bbox_dict({"west": 1, "south": 2, "east": 3, "north": 4}, crs="EPSG:4326") == { "west": 1, "south": 2, @@ -759,6 +780,13 @@ def test_to_bbox_dict_from_dict(self): "north": 4, "crs": 4326, } + assert to_bbox_dict({"west": 1, "south": 2, "east": 3, "north": 4}, crs="4326") == { + "west": 1, + "south": 2, + "east": 3, + "north": 4, + "crs": 4326, + } assert to_bbox_dict( { "west": 1, From dacff114c02302c26c2f87e696d95031ae419d94 Mon Sep 17 00:00:00 2001 From: Johan Schreurs Date: Thu, 17 Aug 2023 22:15:52 +0200 Subject: [PATCH 2/4] Issue #437 Small correction in 'seealso' references --- openeo/rest/datacube.py | 4 ++-- openeo/util.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/openeo/rest/datacube.py b/openeo/rest/datacube.py index 83bba992f..ede0adee6 100644 --- a/openeo/rest/datacube.py +++ b/openeo/rest/datacube.py @@ -328,7 +328,7 @@ def filter_bbox( For example, the following string and int values all specify to the same CRS: ``"EPSG:4326"``, ``"4326"``, and the integer ``4326``. - .. seealso:: `openeo.org documentation on openeo.util.normalize_crs `_ for the most up to date information. + .. seealso:: `openEO Python Client documentation on openeo.util.normalize_crs `_ for the most up to date information. """ if args and any(k is not None for k in (west, south, east, north, bbox)): raise ValueError("Don't mix positional arguments with keyword arguments.") @@ -824,7 +824,7 @@ def _get_geometry_argument( :param crs: data structure that encodes a CRS, typically just an int or string value. If the ``pyproj`` library is available, everything supported by it is allowed. - .. seealso:: `openeo.org documentation on openeo.util.normalize_crs `_ for the most up to date information. + .. seealso:: `openEO Python Client documentation on openeo.util.normalize_crs `_ for the most up to date information. """ if isinstance(geometry, (str, pathlib.Path)): diff --git a/openeo/util.py b/openeo/util.py index 067206423..ff1cc7f9b 100644 --- a/openeo/util.py +++ b/openeo/util.py @@ -547,7 +547,7 @@ class BBoxDict(dict): For example, the following string and int values all specify to the same CRS: ``"EPSG:4326"``, ``"4326"``, and the integer ``4326``. - .. seealso:: `openeo.org documentation on openeo.util.normalize_crs `_ for the most up to date information. + .. seealso:: `openEO Python Client documentation on openeo.util.normalize_crs `_ for the most up to date information. .. versionadded:: 0.10.1 """ From 104129e207959787dad4e3e33c91faa1a7aeea05 Mon Sep 17 00:00:00 2001 From: Johan Schreurs Date: Fri, 18 Aug 2023 13:07:56 +0200 Subject: [PATCH 3/4] Issue #437 Skip new test cases that require higher pyproj/python version when version is lower --- tests/test_util.py | 50 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 37 insertions(+), 13 deletions(-) diff --git a/tests/test_util.py b/tests/test_util.py index e754797be..15406bcac 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -701,6 +701,14 @@ def test_init(self): "north": 4, "crs": 4326, } + + @pytest.mark.skipif( + # TODO #460 this skip is only necessary for python 3.6 and lower + pyproj.__version__ < ComparableVersion("3.3.1"), + reason="pyproj below 3.3.1 does not support int-like strings", + ) + def test_init_python_for_pyprojv331(self): + """Extra test case that does not work with old pyproj versions that we get on python version 3.7 and below.""" assert BBoxDict(west=1, south=2, east=3, north=4, crs="4326") == { "west": 1, "south": 2, @@ -734,6 +742,14 @@ def test_to_bbox_dict_from_sequence(self): "north": 4, "crs": 4326, } + + @pytest.mark.skipif( + # TODO #460 this skip is only necessary for python 3.6 and lower + pyproj.__version__ < ComparableVersion("3.3.1"), + reason="pyproj below 3.3.1 does not support int-like strings", + ) + def test_to_bbox_dict_from_sequence_pyprojv331(self): + """Extra test cases that do not work with old pyproj versions that we get on python version 3.7 and below.""" assert to_bbox_dict([1, 2, 3, 4], crs="4326") == { "west": 1, "south": 2, @@ -766,14 +782,33 @@ def test_to_bbox_dict_from_dict(self): "north": 4, "crs": 4326, } - assert to_bbox_dict({"west": 1, "south": 2, "east": 3, "north": 4, "crs": "4326"}) == { + assert to_bbox_dict({"west": 1, "south": 2, "east": 3, "north": 4}, crs="EPSG:4326") == { "west": 1, "south": 2, "east": 3, "north": 4, "crs": 4326, } - assert to_bbox_dict({"west": 1, "south": 2, "east": 3, "north": 4}, crs="EPSG:4326") == { + assert to_bbox_dict( + { + "west": 1, + "south": 2, + "east": 3, + "north": 4, + "crs": "EPSG:4326", + "color": "red", + "other": "garbage", + } + ) == {"west": 1, "south": 2, "east": 3, "north": 4, "crs": 4326} + + @pytest.mark.skipif( + # TODO #460 this skip is only necessary for python 3.6 and lower + pyproj.__version__ < ComparableVersion("3.3.1"), + reason="pyproj below 3.3.1 does not support int-like strings", + ) + def test_to_bbox_dict_from_dict_for_pyprojv331(self): + """Extra test cases that do not work with old pyproj versions that we get on python version 3.7 and below.""" + assert to_bbox_dict({"west": 1, "south": 2, "east": 3, "north": 4, "crs": "4326"}) == { "west": 1, "south": 2, "east": 3, @@ -787,17 +822,6 @@ def test_to_bbox_dict_from_dict(self): "north": 4, "crs": 4326, } - assert to_bbox_dict( - { - "west": 1, - "south": 2, - "east": 3, - "north": 4, - "crs": "EPSG:4326", - "color": "red", - "other": "garbage", - } - ) == {"west": 1, "south": 2, "east": 3, "north": 4, "crs": 4326} def test_to_bbox_dict_from_dict_missing_field(self): with pytest.raises(InvalidBBoxException, match=re.escape("Missing bbox fields ['north', 'south', 'west']")): From 2c9a133b7733c498c18329e33bf0175ca05edaf2 Mon Sep 17 00:00:00 2001 From: Johan Schreurs Date: Mon, 4 Sep 2023 18:11:20 +0200 Subject: [PATCH 4/4] Issue #437 Improve docstrings based on code review feedback --- openeo/rest/datacube.py | 23 ++++++++++++----------- openeo/util.py | 27 +++++++++++++-------------- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/openeo/rest/datacube.py b/openeo/rest/datacube.py index ede0adee6..734fdfe8f 100644 --- a/openeo/rest/datacube.py +++ b/openeo/rest/datacube.py @@ -308,9 +308,12 @@ def filter_bbox( >>> cube.filter_bbox(bbox_param) >>> cube.filter_bbox(bbox=bbox_param) - - With a CRS other than EPSG 4326 (**see also parameters: crs**):: + - With a CRS other than EPSG 4326:: - >>> cube.filter_bbox(west=652000, east=672000, north=5161000, south=5181000, crs=32632) + >>> cube.filter_bbox( + ... west=652000, east=672000, north=5161000, south=5181000, + ... crs=32632 + ... ) - Deprecated: positional arguments are also supported, but follow a non-standard order for legacy reasons:: @@ -318,17 +321,16 @@ def filter_bbox( >>> west, east, north, south = 3, 4, 52, 51 >>> cube.filter_bbox(west, east, north, south) - :param crs: data structure that encodes a CRS, typically just an int or string value. - If the ``pyproj`` library is available, everything supported by it is allowed. + :param crs: value that encodes a coordinate reference system, typically just an int (EPSG code) or string (authority string). - Integers always refer to the EPSG code that has that number assigned. - A string that contains only an integer it is treated the same way. + Integers always refer to the corresponding EPSG code. + A string that contains only an integer is interpreted as that same integer EPSG code. - You can also specify EPSG codes as a formatted string, "EPSG:number". + You can also specify EPSG codes as an authority string, such as "EPSG:4326". For example, the following string and int values all specify to the same CRS: ``"EPSG:4326"``, ``"4326"``, and the integer ``4326``. - .. seealso:: `openEO Python Client documentation on openeo.util.normalize_crs `_ for the most up to date information. + .. seealso:: openEO Python Client documentation on openeo.util.normalize_crs :py:func:`openeo.util.normalize_crs` for the most up to date information. """ if args and any(k is not None for k in (west, south, east, north, bbox)): raise ValueError("Don't mix positional arguments with keyword arguments.") @@ -822,9 +824,8 @@ def _get_geometry_argument( """ Convert input to a geometry as "geojson" subtype object. - :param crs: data structure that encodes a CRS, typically just an int or string value. - If the ``pyproj`` library is available, everything supported by it is allowed. - .. seealso:: `openEO Python Client documentation on openeo.util.normalize_crs `_ for the most up to date information. + :param crs: value that encodes a coordinate reference system, typically just an int (EPSG code) or string (authority string). + .. seealso:: openEO Python Client documentation on openeo.util.normalize_crs :py:func:`openeo.util.normalize_crs` for the most up to date information. """ if isinstance(geometry, (str, pathlib.Path)): diff --git a/openeo/util.py b/openeo/util.py index ff1cc7f9b..d5b46e3e2 100644 --- a/openeo/util.py +++ b/openeo/util.py @@ -537,17 +537,16 @@ class BBoxDict(dict): (having keys "west", "south", "east", "north", and optionally "crs"). crs: - data structure that encodes a CRS, typically just an int or string value. - If the ``pyproj`` library is available, everything supported by it is allowed. + value that encodes a coordinate reference system, typically just an int (EPSG code) or string (authority string). - Integers always refer to the EPSG code that has that number assigned. - A string that contains only an integer it is treated the same way. + Integers always refer to the corresponding EPSG code. + A string that contains only an integer is interpreted as that same integer EPSG code. - You can also specify EPSG codes as a formatted string, "EPSG:number". + You can also specify EPSG codes as an authority string, such as "EPSG:4326". For example, the following string and int values all specify to the same CRS: ``"EPSG:4326"``, ``"4326"``, and the integer ``4326``. - .. seealso:: `openEO Python Client documentation on openeo.util.normalize_crs `_ for the most up to date information. + .. seealso:: openEO Python Client documentation on openeo.util.normalize_crs :py:func:`openeo.util.normalize_crs` for the most up to date information. .. versionadded:: 0.10.1 """ @@ -663,26 +662,26 @@ def normalize_crs(crs: Any, *, use_pyproj: bool = True) -> Union[None, int, str] Behavior and data structure support depends on the availability of the ``pyproj`` library: - If the ``pyproj`` library is available: use that to do parsing and conversion. - This means that anything that is supported by ``pyproj.CRS.from_user_input`` is allowed. + This means that everything supported by `pyproj.CRS.from_user_input `_ is allowed. See the ``pyproj`` docs for more details. - Otherwise, some best effort validation is done: EPSG looking int/str values will be parsed as such, other strings will be assumed to be WKT2 already. Other data structures will not be accepted. - :param crs: data structure that encodes a CRS, typically just an int or string value. - If the ``pyproj`` library is available, everything supported by it is allowed. - - Integers always refer to the EPSG code that has that number assigned. - A string that contains only an integer it is treated the same way. + Integers always refer to the corresponding EPSG code. + A string that contains only an integer is interpreted as that same integer EPSG code. - You can also specify EPSG codes as a formatted string, "EPSG:number". + You can also specify EPSG codes as an authority string, such as "EPSG:4326". For example, the following string and int values all specify to the same CRS: ``"EPSG:4326"``, ``"4326"``, and the integer ``4326``. + :param crs: value that encodes a coordinate reference system, typically just an int (EPSG code) or string (authority string). + If the ``pyproj`` library is available, everything supported by it is allowed. + :param use_pyproj: whether ``pyproj`` should be leveraged at all (mainly useful for testing the "no pyproj available" code path) - :return: EPSG code as int, or WKT2 string. Or None if input was empty . + :return: EPSG code as int, or WKT2 string. Or None if input was empty. :raises ValueError: When the given CRS data can not be parsed/converted/normalized.