-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[core] Fix get_bounds behavior in core #2012
Conversation
Also: - added alias for getBounds, - fixed an issue where geemap crashed without a helpful message if the map hadn't been added to the screen, and - added some unit tests. The behavior was correct in geemap non-core.
for more information, see https://pre-commit.ci
geemap/core.py
Outdated
@@ -488,9 +488,29 @@ def set_zoom(self, value: int) -> None: | |||
def get_center(self) -> Sequence: | |||
return self.center | |||
|
|||
def get_bounds(self) -> Sequence: | |||
def get_bounds(self, as_geo_json: bool = False) -> Sequence: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we change as_geo_json
to as_geojson
?
geemap/core.py
Outdated
# https://ipyleaflet.readthedocs.io/en/latest/map_and_basemaps/map.html#ipyleaflet.Map.fit_bounds | ||
coords = [bounds[0][1], bounds[0][0], bounds[1][1], bounds[1][0]] | ||
|
||
if as_geo_json: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as_geojson
?
return coords | ||
|
||
getBounds = get_bounds | ||
return super().get_bounds(as_geo_json=asGeoJSON) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as_geojson
?
tests/test_core.py
Outdated
|
||
mock_bounds.__get__ = Mock(return_value=()) | ||
with self.assertRaisesRegex(RuntimeError, "Map bounds are undefined"): | ||
self.core_map.get_bounds(as_geo_json=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as_geojson
?
geemap/core.py
Outdated
"""Returns the bounds of the current map view. | ||
|
||
Args: | ||
as_geo_json (bool, optional): If true, returns map bounds as |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as_geojson
?
Also:
The behavior was correct in geemap non-core.