-
-
Notifications
You must be signed in to change notification settings - Fork 113
Automatically convert xlim/ylim to Web Mercator when tiles=True #1685
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
base: main
Are you sure you want to change the base?
Changes from 9 commits
289c5c1
be6a030
faf4428
d1d6e6b
f55a2e4
3db249d
5f503b5
de2de9d
5f4f4a8
51026d6
fa13680
03d6021
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,7 +47,7 @@ | |
| from holoviews.plotting.util import process_cmap | ||
| from holoviews.operation import histogram, apply_when | ||
| from holoviews.streams import Buffer, Pipe | ||
| from holoviews.util.transform import dim, lon_lat_to_easting_northing | ||
| from holoviews.util.transform import dim | ||
| from pandas import DatetimeIndex, MultiIndex | ||
|
|
||
| from .backend_transforms import _transfer_opts_cur_backend | ||
|
|
@@ -82,6 +82,9 @@ | |
| import_geoviews, | ||
| is_mpl_cmap, | ||
| _find_stack_level, | ||
| is_within_latlon_bounds, | ||
| convert_latlon_to_mercator, | ||
| convert_limit_to_mercator, | ||
| ) | ||
| from .utilities import hvplot_extension | ||
|
|
||
|
|
@@ -1003,6 +1006,17 @@ def __init__( | |
| elif projection is False: | ||
| # to disable automatic projection of tiles | ||
| self.output_projection = projection | ||
| elif tiles and not self.geo and (xlim or ylim): | ||
| should_convert = ( | ||
| not is_geodataframe(data) | ||
| and x is not None | ||
| and y is not None | ||
| and is_within_latlon_bounds(data, x, y) | ||
| ) | ||
|
|
||
| if should_convert: | ||
| xlim = convert_limit_to_mercator(xlim, is_x_axis=True) | ||
| ylim = convert_limit_to_mercator(ylim, is_x_axis=False) | ||
|
|
||
| # Operations | ||
| if resample_when is not None and not any([rasterize, datashade, downsample]): | ||
|
|
@@ -2529,26 +2543,16 @@ def _process_tiles_without_geo(self, data, x, y): | |
| elif is_geodataframe(data): | ||
| if getattr(data, 'crs', None) is not None: | ||
| data = data.to_crs(epsg=3857) | ||
| else: | ||
| min_x = np.min(data[x]) | ||
| max_x = np.max(data[x]) | ||
| min_y = np.min(data[y]) | ||
| max_y = np.max(data[y]) | ||
|
|
||
| x_within_bounds = -180 <= min_x <= 360 and -180 <= max_x <= 360 | ||
| y_within_bounds = -90 <= min_y <= 90 and -90 <= max_y <= 90 | ||
| if x_within_bounds and y_within_bounds: | ||
| data = data.copy() | ||
| lons_180 = (data[x] + 180) % 360 - 180 # ticks are better with -180 to 180 | ||
| easting, northing = lon_lat_to_easting_northing(lons_180, data[y]) | ||
| new_x = 'x' if 'x' not in data else 'x_' # quick existing var check | ||
| new_y = 'y' if 'y' not in data else 'y_' | ||
| data[new_x] = easting | ||
| data[new_y] = northing | ||
| if is_xarray(data): | ||
| data = data.swap_dims({x: new_x, y: new_y}) | ||
| x = new_x | ||
| y = new_y | ||
| elif is_within_latlon_bounds(data, x, y): | ||
| data = data.copy() | ||
| easting, northing = convert_latlon_to_mercator(data[x], data[y]) | ||
| new_x = 'x' if 'x' not in data else 'x_' | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While we're at it, maybe we can do better than that here with a small utility function that finds a field name not already used (e.g. appending underscores until it finds it's available). In this case we don't check if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not too convinced we need this; I personally have not encountered coordinates with |
||
| new_y = 'y' if 'y' not in data else 'y_' | ||
| data[new_x] = easting | ||
| data[new_y] = northing | ||
| if is_xarray(data): | ||
| data = data.swap_dims({x: new_x, y: new_y}) | ||
| x, y = new_x, new_y | ||
| return data, x, y | ||
|
|
||
| def chart(self, element, x, y, data=None): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -104,3 +104,147 @@ def test_plot_without_crs(self): | |
| assert isinstance(plot.get(1), hv.Polygons) | ||
| bk_plot = bk_renderer.get_plot(plot) | ||
| assert bk_plot.projection == 'mercator' # projection enabled due to `tiles=True` | ||
|
|
||
| def test_xlim_ylim_conversion_with_tiles(self, simple_df): | ||
| """Test that xlim and ylim are automatically converted to Web Mercator when tiles=True""" | ||
| from holoviews.util.transform import lon_lat_to_easting_northing | ||
|
||
|
|
||
| # Create a dataframe with lat/lon-like data | ||
| df = pd.DataFrame( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest using a fixture instead of repeating in each function the same DataFrame definition. |
||
| {'lon': [-120.0, -100.0, -80.0], 'lat': [30.0, 35.0, 40.0], 'value': [1, 2, 3]} | ||
| ) | ||
|
|
||
| # Plot with xlim and ylim in lat/lon coordinates | ||
| plot = df.hvplot.points('lon', 'lat', tiles=True, xlim=(-130, -70), ylim=(25, 45)) | ||
|
|
||
| # Get the Points element from the overlay | ||
|
||
| points = plot.get(1) | ||
|
|
||
| # Check that the coordinates were converted | ||
| assert 'x' in points.data.columns or 'x_' in points.data.columns | ||
ahuang11 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| assert 'y' in points.data.columns or 'y_' in points.data.columns | ||
|
|
||
| # Calculate expected xlim and ylim in Web Mercator | ||
| xlim_expected_0, _ = lon_lat_to_easting_northing(-130, 0) | ||
| xlim_expected_1, _ = lon_lat_to_easting_northing(-70, 0) | ||
| _, ylim_expected_0 = lon_lat_to_easting_northing(0, 25) | ||
| _, ylim_expected_1 = lon_lat_to_easting_northing(0, 45) | ||
|
|
||
| # Get the plot opts | ||
| bk_plot = bk_renderer.get_plot(plot) | ||
|
|
||
| # Check that xlim and ylim were converted | ||
| # The plot should have x_range and y_range set to the converted values | ||
| assert hasattr(bk_plot.handles['plot'], 'x_range') | ||
| assert hasattr(bk_plot.handles['plot'], 'y_range') | ||
|
|
||
| # Verify the ranges are in Web Mercator (much larger values than lat/lon) | ||
| x_range_start = bk_plot.handles['plot'].x_range.start | ||
| x_range_end = bk_plot.handles['plot'].x_range.end | ||
| y_range_start = bk_plot.handles['plot'].y_range.start | ||
| y_range_end = bk_plot.handles['plot'].y_range.end | ||
|
|
||
| # Web Mercator values should be much larger than lat/lon | ||
| # xlim in Web Mercator should be around -14e6 to -7e6 | ||
| assert abs(x_range_start) > 10000000 # Much larger than -130 | ||
| assert abs(x_range_end) > 7000000 # Much larger than -70 | ||
|
|
||
| # ylim in Web Mercator should be around 2.8e6 to 5.6e6 | ||
| assert y_range_start > 2000000 # Much larger than 25 | ||
| assert y_range_end > 5000000 # Much larger than 45 | ||
|
|
||
| # Check that the values are approximately correct | ||
| assert abs(x_range_start - xlim_expected_0) < 100000 | ||
| assert abs(x_range_end - xlim_expected_1) < 100000 | ||
| assert abs(y_range_start - ylim_expected_0) < 100000 | ||
| assert abs(y_range_end - ylim_expected_1) < 100000 | ||
ahuang11 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| def test_xlim_only_conversion_with_tiles(self, simple_df): | ||
| """xlim should convert even when ylim is not provided.""" | ||
|
|
||
| df = pd.DataFrame( | ||
| { | ||
| 'lon': [-120.0, -100.0, -80.0], | ||
| 'lat': [30.0, 35.0, 40.0], | ||
| } | ||
| ) | ||
|
|
||
| plot = df.hvplot.points('lon', 'lat', tiles=True, xlim=(-130, -70)) | ||
| bk_plot = bk_renderer.get_plot(plot) | ||
|
|
||
| x_start = bk_plot.state.x_range.start | ||
| x_end = bk_plot.state.x_range.end | ||
|
|
||
| assert abs(x_start) > 10_000_000 | ||
| assert abs(x_end) > 7_000_000 | ||
| assert x_start < x_end | ||
|
|
||
| def test_ylim_only_conversion_with_tiles(self, simple_df): | ||
| """ylim should convert even when xlim is not provided.""" | ||
|
|
||
| df = pd.DataFrame( | ||
| { | ||
| 'lon': [-120.0, -100.0, -80.0], | ||
| 'lat': [30.0, 35.0, 40.0], | ||
| } | ||
| ) | ||
|
|
||
| plot = df.hvplot.points('lon', 'lat', tiles=True, ylim=(25, 45)) | ||
| bk_plot = bk_renderer.get_plot(plot) | ||
|
|
||
| y_start = bk_plot.state.y_range.start | ||
| y_end = bk_plot.state.y_range.end | ||
|
|
||
| assert y_start > 2_000_000 | ||
| assert y_end > 5_000_000 | ||
| assert y_start < y_end | ||
|
|
||
| def test_xlim_ylim_not_converted_without_tiles(self): | ||
| """Test that xlim and ylim are NOT converted when tiles=False""" | ||
| # Create a dataframe with lat/lon-like data | ||
| df = pd.DataFrame( | ||
| {'lon': [-120.0, -100.0, -80.0], 'lat': [30.0, 35.0, 40.0], 'value': [1, 2, 3]} | ||
| ) | ||
|
|
||
| # Plot without tiles - xlim/ylim should NOT be converted | ||
| plot = df.hvplot.points('lon', 'lat', xlim=(-130, -70), ylim=(25, 45)) | ||
|
|
||
| # Get the plot opts - note: without tiles it's a Points element, not an overlay | ||
| bk_plot = bk_renderer.get_plot(plot) | ||
|
|
||
| # Check that xlim and ylim were NOT converted (still in lat/lon range) | ||
| x_range_start = bk_plot.handles['plot'].x_range.start | ||
| x_range_end = bk_plot.handles['plot'].x_range.end | ||
| y_range_start = bk_plot.handles['plot'].y_range.start | ||
| y_range_end = bk_plot.handles['plot'].y_range.end | ||
|
|
||
| # Values should still be in lat/lon range | ||
| assert -140 < x_range_start < -120 | ||
| assert -80 < x_range_end < -60 | ||
| assert 20 < y_range_start < 30 | ||
| assert 40 < y_range_end < 50 | ||
|
|
||
| def test_xlim_ylim_out_of_bounds_not_converted(self): | ||
| """Test that xlim and ylim are NOT converted when values are outside lat/lon bounds""" | ||
| # Create a dataframe with arbitrary data | ||
| df = pd.DataFrame( | ||
| {'x': [1000.0, 2000.0, 3000.0], 'y': [500.0, 600.0, 700.0], 'value': [1, 2, 3]} | ||
| ) | ||
|
|
||
| # Plot with tiles but xlim/ylim outside lat/lon bounds | ||
| plot = df.hvplot.points('x', 'y', tiles=True, xlim=(1000, 3000), ylim=(400, 800)) | ||
|
|
||
| # Get the plot opts | ||
| bk_plot = bk_renderer.get_plot(plot.get(1)) | ||
|
|
||
| # Check that xlim and ylim were NOT converted (still in original range) | ||
| x_range_start = bk_plot.handles['plot'].x_range.start | ||
| x_range_end = bk_plot.handles['plot'].x_range.end | ||
| y_range_start = bk_plot.handles['plot'].y_range.start | ||
| y_range_end = bk_plot.handles['plot'].y_range.end | ||
|
|
||
| # Values should still be in original range (not Web Mercator) | ||
| assert 900 < x_range_start < 1100 | ||
ahuang11 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| assert 2900 < x_range_end < 3100 | ||
| assert 300 < y_range_start < 500 | ||
| assert 700 < y_range_end < 900 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please add unit tests for these functions in |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,7 @@ | |
| import pandas as pd | ||
| import param | ||
| import holoviews as hv | ||
| from holoviews.util.transform import lon_lat_to_easting_northing | ||
|
|
||
| try: | ||
| import panel as pn | ||
|
|
@@ -410,6 +411,50 @@ def process_crs(crs): | |
| ) from Exception(*errors) | ||
|
|
||
|
|
||
| def is_within_latlon_bounds(data, x, y): | ||
|
||
| """Return True when finite lat/lon bounds are detected.""" | ||
| try: | ||
| min_x = np.min(data[x]) | ||
| max_x = np.max(data[x]) | ||
| min_y = np.min(data[y]) | ||
| max_y = np.max(data[y]) | ||
| except (KeyError, ValueError, TypeError): | ||
|
||
| return False | ||
|
|
||
| x_ok = -180 <= min_x <= 360 and -180 <= max_x <= 360 | ||
| y_ok = -90 <= min_y <= 90 and -90 <= max_y <= 90 | ||
| return bool(x_ok and y_ok) | ||
ahuang11 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
|
|
||
| def convert_latlon_to_mercator(lon, lat): | ||
| """Convert lon/lat values to Web Mercator easting/northing.""" | ||
| lon_normalized = (lon + 180) % 360 - 180 | ||
ahuang11 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return lon_lat_to_easting_northing(lon_normalized, lat) | ||
|
|
||
|
|
||
| def convert_limit_to_mercator(limit, is_x_axis=True): | ||
| """Convert axis limits to Web Mercator coordinates when possible.""" | ||
|
|
||
| if not limit: | ||
| return None | ||
|
|
||
| try: | ||
| v0, v1 = limit | ||
| except (TypeError, ValueError): | ||
ahuang11 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| return limit | ||
|
|
||
| if is_x_axis: | ||
ahuang11 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| if not (-180 <= v0 <= 360 and -180 <= v1 <= 360): | ||
| return limit | ||
| (v0_merc, v1_merc), _ = convert_latlon_to_mercator(np.array([v0, v1]), (0, 0)) | ||
| else: | ||
| if not (-90 <= v0 <= 90 and -90 <= v1 <= 90): | ||
| return limit | ||
| _, (v0_merc, v1_merc) = convert_latlon_to_mercator(np.array([0, 0]), (v0, v1)) | ||
|
|
||
| return (v0_merc, v1_merc) | ||
|
|
||
|
|
||
| def is_list_like(obj): | ||
| """ | ||
| Adapted from pandas' is_list_like cython function. | ||
|
|
||
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.
Not sure we should document things this way in the gallery. Which is why I suggested doing it in the
tilesexample section, as this behavior is only enabled whentilesis enabled. For this example, I simply suggest updating the original snippet withxlim=..., ylim=....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.
Thanks for clarifying, that makes sense. I wasn’t really sure what “a real example in the tiles section” referred to since I haven't worked in hvplot docs for a while, so just a heads up a quick link to the hvPlot docs section would be helpful for future comments / contributors! 🙇🏻
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.
Good point! 👍