Remove the invalid default CRS #233
Conversation
m-mohr
force-pushed
the
remove_invalid_default_crs
branch
5 times, most recently
from
b4309bd
to
faae692
Compare
m-mohr
force-pushed
the
remove_invalid_default_crs
branch
from
faae692
to
7407af6
Compare
|
Getting rid of invalid But I'm not sure we should remove it completely instead of replacing it with valid int |
|
I've added a non 4326 example to the filter_bbox documentation. If all examples use 4326 and we want some non-default examples, we should probably replace some with non 4326 bboxes as examples instead. |
|
To simplify review, I already cherry-picked a large part of the original PR in 75e4a58 |
| @@ -131,7 +131,7 @@ pixels to 0 and all other pixels to 1 using a simple comparison:: | |||
| s2_sceneclassification = ( | |||
| connection.load_collection("TERRASCOPE_S2_TOC_V2", bands=["SCENECLASSIFICATION_20M"]) | |||
| .filter_temporal(extent=["2016-01-01", "2016-03-10"]) | |||
| .filter_bbox(west=5.1518, east=5.1533,south=51.1819,north=51.1846, crs=4326) | |||
| .filter_bbox(west=5.1518, east=5.1533,south=51.1819,north=51.1846) | |||
There was a problem hiding this comment.
I would keep these two cases of crs 4326 because of its documentation value
There was a problem hiding this comment.
I guess it would make sense to describe somewhere that 4326 is the default and can be omitted - or use another CRS for the example.
There was a problem hiding this comment.
yeah that line (and other lines) "need work" anyway: you have to scroll horizontally to see everything on https://open-eo.github.io/openeo-python-client/basics.html#example-applying-a-mask
| } | ||
| if crs is not None: | ||
| extent.update(crs=crs) |
There was a problem hiding this comment.
There should be a bit of test coverage of this (and on datacube.py too)
|
fyi: I'll need to get back to this after my vacation, so this will stay open and unresolved for about 3 weeks. |
|
No problem, I kept some things open because I wanted to improve unit test coverage of them first |
|
The remaining bits are now also incorporated in (1fe248f) (closes this PR) |
Remove
crs = "EPSG:4326"from examples due to the following reasons:4326) is the default CRS anyway, so it is redundant.Also, the Python client should not send the crs as null.
I have not updated the Notebooks.