From 89c7b2510668001f4d07f47131441ea99c617fe9 Mon Sep 17 00:00:00 2001 From: dfguerrerom Date: Wed, 22 May 2024 13:02:43 +0200 Subject: [PATCH 1/6] feat: Make a lazy loading of gee gdf..close #919. --- sepal_ui/aoi/aoi_model.py | 73 +++++++++++++++++++++++++-------- sepal_ui/aoi/aoi_view.py | 6 ++- sepal_ui/message/en/locale.json | 3 +- 3 files changed, 62 insertions(+), 20 deletions(-) diff --git a/sepal_ui/aoi/aoi_model.py b/sepal_ui/aoi/aoi_model.py index aa1921f0..60e12503 100644 --- a/sepal_ui/aoi/aoi_model.py +++ b/sepal_ui/aoi/aoi_model.py @@ -249,10 +249,6 @@ def _from_asset(self, asset_json: dict) -> Self: # set the feature collection self.feature_collection = ee_col - # create a gdf form the feature_collection - features = self.feature_collection.getInfo()["features"] - self.gdf = gpd.GeoDataFrame.from_features(features).set_crs(epsg=4326) - return self def _from_points(self, point_json: dict) -> Self: @@ -377,20 +373,30 @@ def _from_admin(self, admin: str) -> Self: # pygaul needs extra work as ISO codes are not included in the GEE dataset if self.gee: self.feature_collection = pygaul.AdmItems(admin=admin) - features = self.feature_collection.getInfo()["features"] - self.gdf = gpd.GeoDataFrame.from_features(features).set_crs(epsg=4326) - gaul_country = str(self.gdf.ADM0_CODE.unique()[0]) - iso = json.loads(self.MAPPING.read_text())[gaul_country] - self.gdf["ISO"] = iso + + # get the ADM0_CODE to get the ISO code + feature = self.feature_collection.first() + iso = json.loads(self.MAPPING.read_text())[str(feature.get("ADM0_CODE").getInfo())] + + names = ( + (feature.propertyNames().filter(ee.Filter.stringContains("item", "NAME"))) + .map(lambda col_name: feature.get(col_name)) + .getInfo() + ) + + # generate the name from the columns + names = [su.normalize_str(name) for name in names] + names[0] = iso + self.name = "_".join(names) else: self.gdf = pygadm.AdmItems(admin=admin) - # generate the name from the columns - r = self.gdf.iloc[0] - names = [su.normalize_str(r[c]) for c in self.gdf.columns if "NAME" in c] - names[0] = r.ISO if self.gee else r.GID_0[:3] - self.name = "_".join(names) + # generate the name from the columns + r = self.gdf.iloc[0] + names = [su.normalize_str(r[c]) for c in self.gdf.columns if "NAME" in c] + names[0] = r.GID_0[:3] + self.name = "_".join(names) return self @@ -433,7 +439,7 @@ def get_columns(self) -> List[str]: Returns: sorted list of column names """ - if self.gdf is None: + if self.gdf is None and not self.feature_collection: raise Exception(ms.aoi_sel.exception.no_gdf) if self.gee: @@ -455,7 +461,7 @@ def get_fields(self, column: str) -> List[str]: sorted list of fields value """ - if self.gdf is None: + if self.gdf is None and not self.feature_collection: raise Exception(ms.aoi_sel.exception.no_gdf) if self.gee: @@ -476,7 +482,7 @@ def get_selected(self, column: str, field: str) -> Union[ee.Feature, gpd.GeoData Returns: The Feature associated with the query """ - if self.gdf is None: + if self.gdf is None and not self.feature_collection: raise Exception(ms.aoi_sel.exception.no_gdf) if self.gee: @@ -492,9 +498,13 @@ def total_bounds(self) -> Tuple[float, float, float, float]: Returns: minxx, miny, maxx, maxy """ - if self.gdf is None: + if self.gdf is None and not self.feature_collection: raise ValueError(ms.aoi_sel.exception.no_gdf) + if self.gee: + coords = self.feature_collection.geometry().bounds().coordinates().get(0).getInfo() + return [coords[0][0], coords[0][1], coords[3][0], coords[3][1]] + return self.gdf.total_bounds.tolist() def export_to_asset(self) -> Self: @@ -553,3 +563,30 @@ def get_ipygeojson(self, style: Optional[dict] = None) -> GeoJSON: self.ipygeojson = GeoJSON(data=data, style=style, name="aoi") return self.ipygeojson + + @property + def gdf(self): + """Get the geodataframe associated with the AOI.""" + if self.gee: + if not self.feature_collection: + return None + + self._load_gdf() + + return self._gdf + + @gdf.setter + def gdf(self, value): + """Set the gdf value. Used mainly to reset the gdf value.""" + self._gdf = value + + def _load_gdf(self): + """Return a geodataframe from a feature collection.""" + features = self.feature_collection.getInfo()["features"] + self._gdf = gpd.GeoDataFrame.from_features(features).set_crs(epsg=4326) + + if self.method in ["ADMIN0", "ADMIN1", "ADMIN2"]: + + gaul_country = str(self._gdf.ADM0_CODE.unique()[0]) + iso = json.loads(self.MAPPING.read_text())[gaul_country] + self._gdf["ISO"] = iso diff --git a/sepal_ui/aoi/aoi_view.py b/sepal_ui/aoi/aoi_view.py index 601411a3..41bb7483 100644 --- a/sepal_ui/aoi/aoi_view.py +++ b/sepal_ui/aoi/aoi_view.py @@ -349,7 +349,11 @@ def _update_aoi(self, *args) -> Self: if self.map_: self.map_.remove_layer("aoi", none_ok=True) self.map_.zoom_bounds(self.model.total_bounds()) - self.map_.add_layer(self.model.get_ipygeojson(self.map_style)) + + if self.gee: + self.map_.add_ee_layer(self.model.feature_collection, {}, "aoi") + else: + self.map_.add_layer(self.model.get_ipygeojson(self.map_style), "aoi") self.aoi_dc.hide() diff --git a/sepal_ui/message/en/locale.json b/sepal_ui/message/en/locale.json index f03349a2..71e0d4c9 100644 --- a/sepal_ui/message/en/locale.json +++ b/sepal_ui/message/en/locale.json @@ -80,7 +80,8 @@ "no_draw": "Please draw a shape in the map", "no_admlyr": "Select an administrative layer", "invalid_code": "The code is not in the database", - "no_gdf": "You must set the gdf before interacting with it" + "no_gdf": "You must set the gdf before interacting with it", + "no_fc": "You have to select a feature collection first" } }, "mapping": { From f82cc9114edada2a17cf9367b7054d4956527b7b Mon Sep 17 00:00:00 2001 From: dfguerrerom Date: Wed, 22 May 2024 13:43:46 +0200 Subject: [PATCH 2/6] refactor: round total bounds --- sepal_ui/aoi/aoi_model.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/sepal_ui/aoi/aoi_model.py b/sepal_ui/aoi/aoi_model.py index 60e12503..5a4c51d5 100644 --- a/sepal_ui/aoi/aoi_model.py +++ b/sepal_ui/aoi/aoi_model.py @@ -503,9 +503,11 @@ def total_bounds(self) -> Tuple[float, float, float, float]: if self.gee: coords = self.feature_collection.geometry().bounds().coordinates().get(0).getInfo() - return [coords[0][0], coords[0][1], coords[3][0], coords[3][1]] + bounds = [coords[0][0], coords[0][1], coords[3][0], coords[3][1]] + else: + bounds = self.gdf.total_bounds.tolist() - return self.gdf.total_bounds.tolist() + return [round(bound, 4) for bound in bounds] def export_to_asset(self) -> Self: """Export the feature_collection as an asset (only for ee model).""" From d81f603839de5cbad70bd178d812687798722572 Mon Sep 17 00:00:00 2001 From: dfguerrerom Date: Wed, 22 May 2024 14:08:33 +0200 Subject: [PATCH 3/6] refactor: only do one call to gee --- sepal_ui/aoi/aoi_model.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/sepal_ui/aoi/aoi_model.py b/sepal_ui/aoi/aoi_model.py index 5a4c51d5..e6dd4d28 100644 --- a/sepal_ui/aoi/aoi_model.py +++ b/sepal_ui/aoi/aoi_model.py @@ -376,17 +376,15 @@ def _from_admin(self, admin: str) -> Self: # get the ADM0_CODE to get the ISO code feature = self.feature_collection.first() - iso = json.loads(self.MAPPING.read_text())[str(feature.get("ADM0_CODE").getInfo())] + properties = feature.toDictionary(feature.propertyNames()).getInfo() - names = ( - (feature.propertyNames().filter(ee.Filter.stringContains("item", "NAME"))) - .map(lambda col_name: feature.get(col_name)) - .getInfo() - ) + iso = json.loads(self.MAPPING.read_text())[str(properties.get("ADM0_CODE"))] + names = [value for prop, value in properties.items() if "NAME" in prop] # generate the name from the columns names = [su.normalize_str(name) for name in names] names[0] = iso + self.name = "_".join(names) else: From b67c88ac4eadeba3b54c356b15cb815541773ec0 Mon Sep 17 00:00:00 2001 From: dfguerrerom Date: Thu, 30 May 2024 14:03:36 +0200 Subject: [PATCH 4/6] refactor(aoi_model): - set gdf when vector or geojson is set in method. - don't evaluate gdf in conditions to avoid request when using gee. --- sepal_ui/aoi/aoi_model.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/sepal_ui/aoi/aoi_model.py b/sepal_ui/aoi/aoi_model.py index e6dd4d28..feef30d9 100644 --- a/sepal_ui/aoi/aoi_model.py +++ b/sepal_ui/aoi/aoi_model.py @@ -395,7 +395,6 @@ def _from_admin(self, admin: str) -> Self: names = [su.normalize_str(r[c]) for c in self.gdf.columns if "NAME" in c] names[0] = r.GID_0[:3] self.name = "_".join(names) - return self def clear_output(self) -> Self: @@ -437,7 +436,7 @@ def get_columns(self) -> List[str]: Returns: sorted list of column names """ - if self.gdf is None and not self.feature_collection: + if self._gdf is None and not self.feature_collection: raise Exception(ms.aoi_sel.exception.no_gdf) if self.gee: @@ -459,7 +458,7 @@ def get_fields(self, column: str) -> List[str]: sorted list of fields value """ - if self.gdf is None and not self.feature_collection: + if self._gdf is None and not self.feature_collection: raise Exception(ms.aoi_sel.exception.no_gdf) if self.gee: @@ -480,7 +479,7 @@ def get_selected(self, column: str, field: str) -> Union[ee.Feature, gpd.GeoData Returns: The Feature associated with the query """ - if self.gdf is None and not self.feature_collection: + if self._gdf is None and not self.feature_collection: raise Exception(ms.aoi_sel.exception.no_gdf) if self.gee: @@ -496,7 +495,8 @@ def total_bounds(self) -> Tuple[float, float, float, float]: Returns: minxx, miny, maxx, maxy """ - if self.gdf is None and not self.feature_collection: + # use _gdf to evaluate the condition to avoid accessing the gdf property + if self._gdf is None and not self.feature_collection: raise ValueError(ms.aoi_sel.exception.no_gdf) if self.gee: @@ -543,7 +543,8 @@ def get_ipygeojson(self, style: Optional[dict] = None) -> GeoJSON: Returns: The geojson layer of the aoi gdf, ready to use in a Map """ - if self.gdf is None: + # Evaluate _gdf to avoid accessing the gdf property and calculate it + if self._gdf is None: raise Exception(ms.aoi_sel.exception.no_gdf) # read the data from geojson and add the name as a property of the shape @@ -568,6 +569,10 @@ def get_ipygeojson(self, style: Optional[dict] = None) -> GeoJSON: def gdf(self): """Get the geodataframe associated with the AOI.""" if self.gee: + if self._gdf is not None: + # This happens when it comes from vector or geojson + return self._gdf + if not self.feature_collection: return None From dc43e412c7a90a83fa622508c5032cd8daabcbd2 Mon Sep 17 00:00:00 2001 From: dfguerrerom Date: Thu, 30 May 2024 14:07:41 +0200 Subject: [PATCH 5/6] test: add new tests when using aoi_model with gee --- tests/test_aoi/test_AoiModel.py | 49 +++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/tests/test_aoi/test_AoiModel.py b/tests/test_aoi/test_AoiModel.py index dd2f9989..0fa16835 100644 --- a/tests/test_aoi/test_AoiModel.py +++ b/tests/test_aoi/test_AoiModel.py @@ -332,6 +332,34 @@ def test_from_vector(gee_dir: Path, fake_vector: dict) -> None: return +@pytest.mark.skipif(not ee.data._credentials, reason="GEE is not set") +def test_from_vector_gee(gee_dir: Path, fake_vector: dict) -> None: + """Get an AoiModel from a vector and using GEE. + + Args: + gee_dir: the path to the session gee_dir folder (including hash) + fake_vector: the path to a vector file + """ + aoi_model = aoi.AoiModel(folder=gee_dir, gee=True) + + # with no pathname + with pytest.raises(Exception): + aoi_model._from_vector(fake_vector) + + # all params + vector = {"pathname": fake_vector, "column": "GID_0", "value": "VAT"} + aoi_model._from_vector(vector) + assert aoi_model.name == "gadm41_VAT_0_GID_0_VAT" + + # Check that the vector was converted to feature_collection + assert aoi_model.feature_collection is not None + assert aoi_model.feature_collection.first().toDictionary().values().getInfo() == [ + "VaticanCity", + "VAT", + ] + assert aoi_model.gdf is not None + + @pytest.mark.skipif(not ee.data._credentials, reason="GEE is not set") def test_from_geo_json(gee_dir, square: dict) -> None: """Get an AoiModel from a geojson (equivalent to draw). @@ -354,6 +382,27 @@ def test_from_geo_json(gee_dir, square: dict) -> None: return +@pytest.mark.skipif(not ee.data._credentials, reason="GEE is not set") +def test_from_geo_json_gee(gee_dir, square: dict) -> None: + """Get an AoiModel from a geojson (equivalent to draw). + + Args: + gee_dir: the path to the session gee_dir folder (including hash) + square: the geo_interface representation of a quare around vatican + """ + aoi_model = aoi.AoiModel(folder=gee_dir, gee=True) + + # fully qualified square + aoi_model.name = "square" + aoi_model._from_geo_json(square) + assert aoi_model.name == "square" + + # Check the feature collection exists + assert aoi_model.feature_collection.getInfo() + + return + + @pytest.mark.skipif(not ee.data._credentials, reason="GEE is not set") def test_from_asset(gee_dir: Path) -> Path: """Get an AoiModel from gee assets. From b45ab392e13af37a217092ee0b89b244858c13f3 Mon Sep 17 00:00:00 2001 From: dfguerrerom Date: Thu, 30 May 2024 14:43:44 +0200 Subject: [PATCH 6/6] refactor: evaluate gdf in get_ipygeojson since this function is agnostic regarding gee-mode --- sepal_ui/aoi/aoi_model.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sepal_ui/aoi/aoi_model.py b/sepal_ui/aoi/aoi_model.py index feef30d9..66a3166a 100644 --- a/sepal_ui/aoi/aoi_model.py +++ b/sepal_ui/aoi/aoi_model.py @@ -543,8 +543,9 @@ def get_ipygeojson(self, style: Optional[dict] = None) -> GeoJSON: Returns: The geojson layer of the aoi gdf, ready to use in a Map """ - # Evaluate _gdf to avoid accessing the gdf property and calculate it - if self._gdf is None: + # This function aims to work in the same way in both gee and non-gee mode + # It's why we use the gdf property to evaluate the condition + if self.gdf is None: raise Exception(ms.aoi_sel.exception.no_gdf) # read the data from geojson and add the name as a property of the shape