Skip to content
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

Fix x ranges upon projection #722

Merged
merged 8 commits into from
Jun 3, 2024
Merged

Fix x ranges upon projection #722

merged 8 commits into from
Jun 3, 2024

Conversation

ahuang11
Copy link
Collaborator

@ahuang11 ahuang11 commented Apr 12, 2024

Data no longer disappears upon zoom as described in #673 (comment).

Closes #673

Screen.Recording.2024-04-12.at.10.01.04.AM.mov
import xarray as xr
import panel as pn
import holoviews as hv
from holoviews.operation.datashader import rasterize
import geoviews as gv
import cartopy.crs as ccrs

hv.renderer("bokeh").webgl = False
import hvplot.xarray
ds = xr.tutorial.open_dataset("air_temperature").isel(time=0)

rasterize((gv.Image(ds, ["lon", "lat"], ["air"]))) * gv.feature.coastline()

@ahuang11 ahuang11 requested a review from hoxbro April 12, 2024 17:01
Copy link
Member

@jbednar jbednar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing this. I can't quite decide if it's the right fix, though; would be good to have e.g. @philippjfr think about it.

geoviews/plotting/bokeh/callbacks.py Outdated Show resolved Hide resolved
@ahuang11
Copy link
Collaborator Author

ahuang11 commented Apr 12, 2024

My other three thoughts about approaches to fixing this is:

  1. require -180 to 180 for PlateCarree, else raise an error
  2. same as 1, but raise a warning instead
  3. if 0-360 is detected, automatically convert the data (potentially costly to load data)

This PR's fix only updates two integers (the x_range), which is basically "free"

I don't think this can be patched in HoloViews either because the logic here https://github.com/holoviz/holoviews/blob/main/holoviews/operation/resample.py#L139-L140 is correct although I don't know what the difference between x0 and ex0

@TheoMathurin
Copy link

Maybe this can possibly also fix #593?

@ahuang11
Copy link
Collaborator Author

Maybe! If it uses lat/lon, then there's a chance.

Co-authored-by: Philipp Rudiger <prudiger@anaconda.com>
@ahuang11
Copy link
Collaborator Author

ahuang11 commented May 7, 2024

The changes work when it's not overlaid with coastline.

import xarray as xr
import panel as pn
import holoviews as hv
from holoviews.operation.datashader import rasterize
import geoviews as gv
import cartopy.crs as ccrs

hv.extension("bokeh")
import hvplot.xarray
ds = xr.tutorial.open_dataset("air_temperature").isel(time=0)

rasterize(gv.Image(ds, ["lon", "lat"], ["air"]))

However, once coastline is overlaid, it produces a blank image.
image

def _set_unwrap_lons(self, element):
if isinstance(self.geographic, _CylindricalProjection):
x1, x2 = element.range(0)
self._unwrap_lons = 0 <= x1 <= 360 and 0 <= x2 <= 360
Copy link
Member

@philippjfr philippjfr Oct 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is too eager, specifically it should check that it's actually in the 0-360 range rather than merely all being east of Greenwich, so something like:

          self._unwrap_lons = 0 <= x1 <= 360 and 180 <= x2 <= 360

@@ -64,6 +64,10 @@ def project_ranges(cb, msg, attributes):
extents = x0, y0, x1, y1
x0, y0, x1, y1 = project_extents(extents, plot.projection,
plot.current_frame.crs)
if plot._unwrap_lons and -180 <= x0 < 0 or -180 <= x1 < 0:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be:

if plot._unwrap_lons and (-180 <= x0 < 0 or -180 <= x1 < 0):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

geo=True and rasterize=True makes the plot disappear
4 participants