Fix issues with unwrapping longitudes in RangeXY stream #756

philippjfr · 2024-10-16T14:26:16Z

This PR introduced two separate issues: #722

Fixes #753

codecov · 2024-10-16T14:33:29Z

Codecov Report

Attention: Patch coverage is 73.84615% with 17 lines in your changes missing coverage. Please review.

Project coverage is 69.30%. Comparing base (da8719e) to head (238513a).

Files with missing lines	Patch %	Lines
geoviews/tests/ui/test_plot.py	64.58%	17 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #756      +/-   ##
==========================================
+ Coverage   67.40%   69.30%   +1.90%     
==========================================
  Files          45       45              
  Lines        4669     4721      +52     
==========================================
+ Hits         3147     3272     +125     
+ Misses       1522     1449      -73

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚨 Try these New Features:

Flaky Tests Detection - Detect and resolve failed and flaky tests
JS Bundle Analysis - Avoid shipping oversized bundles

geoviews/plotting/bokeh/plot.py

ahuang11 · 2024-10-16T16:23:37Z

Something is still up in this PR

!wget -nc https://downloads.psl.noaa.gov/Datasets/ncep.reanalysis/Monthlies/surface/air.day.ltm.nc
import xarray as xr
import holoviews as hv
from holoviews.operation.datashader import rasterize
import geoviews as gv
gv.extension("bokeh")

ds = xr.open_dataset("air.day.ltm.nc", engine="h5netcdf", decode_times=False).isel(time=0)
rasterize((gv.Image(ds["air"], ["lon", "lat"], ["air"]))) * gv.feature.coastline()

ahuang11 · 2024-10-16T17:02:36Z

I'm starting to think #722 wasn't the right solution because if the data extents are global from 0 to 360, the ranges wrap to -180 to 180, meaning only 0 to 180 remains while 180-360 is cropped and discarded, resulting in half image:

I think we need to either warn and project the lons to -180 to 180 or raise an error mentioning that data must be from -180 to 180 for operations to work properly, i.e.

ds["lon"] = (ds["lon"] + 180) % 360 - 180
ds = ds.sortby("lon")

Separately, there seems to be some padding with the x0/x1:


            self._unwrap_lons = 0 <= x0 <= 360 and 180 <= x1 <= 360
            print(x0, x1, "Xs", self._unwrap_lons)

# outputs
# -1.25 358.75 Xs False
# -1.25 358.75 Xs False

ahuang11 · 2024-10-16T17:45:22Z

Actually, if we extend the range from -180 to 540, it seems to work as expected:

Screen.Recording.2024-10-16.at.10.38.58.AM.mov

Screen.Recording.2024-10-16.at.10.45.50.AM.mov

hoxbro

Can you add test cases for all the cases you have tested locally so we don't get any regressions?

If needed, I can set up UI tests.

ahuang11 · 2024-10-28T14:17:56Z

Yes can you help me set up ui tests or show me a reference?

ahuang11 · 2024-10-28T14:18:03Z

Yes can you help me set up ui tests or show me a reference?

hoxbro · 2024-10-28T15:00:11Z

I have opened a PR with the infrastructure here: #760.

I will also try to fix the CI in it and then merge it.

ahuang11 · 2024-11-22T01:19:04Z

Any ideas how to make test-core skip UI?

hoxbro · 2024-11-22T14:01:44Z

Pushed some updates. I think the main problem was no __init__.py was available in the directory, which confused the test filtering.

Also moved rasterize import into the function, so it does not fail when collecting test on core.

hoxbro · 2024-11-22T14:05:19Z

Looking at the logs it is likely you need to add cartopy naturalearth download to scripts/download_data.py.

https://github.com/holoviz/geoviews/actions/runs/11973974023/job/33384069933?pr=756#step:3:926

edit: did this in dd72524

ahuang11 · 2024-11-22T20:33:18Z

Have you seen this before?

--- Logging error ---
Traceback (most recent call last):
  File "/home/runner/work/geoviews/geoviews/.pixi/envs/test-ui/lib/python3.12/logging/__init__.py", line 1163, in emit
    stream.write(msg + self.terminator)
ValueError: I/O operation on closed file.
Call stack:
  File "/home/runner/work/geoviews/geoviews/.pixi/envs/test-ui/lib/python3.12/threading.py", line 1032, in _bootstrap
    self._bootstrap_inner()
  File "/home/runner/work/geoviews/geoviews/.pixi/envs/test-ui/lib/python3.12/threading.py", line 1075, in _bootstrap_inner
    self.run()
  File "/home/runner/work/geoviews/geoviews/.pixi/envs/test-ui/lib/python3.12/site-packages/panel/io/threads.py", line 28, in run
    bokeh_server = target(*args, **kwargs)
  File "/home/runner/work/geoviews/geoviews/.pixi/envs/test-ui/lib/python3.12/site-packages/panel/io/server.py", line 1078, in get_server
    server.io_loop.start()
  File "/home/runner/work/geoviews/geoviews/.pixi/envs/test-ui/lib/python3.12/site-packages/tornado/platform/asyncio.py", line 205, in start
    self.asyncio_loop.run_forever()
  File "/home/runner/work/geoviews/geoviews/.pixi/envs/test-ui/lib/python3.12/asyncio/base_events.py", line [64](https://github.com/holoviz/geoviews/actions/runs/11979575224/job/33402145460?pr=756#step:3:65)1, in run_forever
    self._run_once()
  File "/home/runner/work/geoviews/geoviews/.pixi/envs/test-ui/lib/python3.12/asyncio/base_events.py", line 1986, in _run_once
    handle._run()
  File "/home/runner/work/geoviews/geoviews/.pixi/envs/test-ui/lib/python3.12/asyncio/events.py", line 88, in _run
    self._context.run(self._callback, *self._args)
  File "/home/runner/work/geoviews/geoviews/.pixi/envs/test-ui/lib/python3.12/site-packages/tornado/ioloop.py", line 939, in _run
    await val
  File "/home/runner/work/geoviews/geoviews/.pixi/envs/test-ui/lib/python3.12/site-packages/bokeh/server/tornado.py", line 725, in _cleanup_sessions
    await app._cleanup_sessions(self._unused_session_lifetime_milliseconds)
  File "/home/runner/work/geoviews/geoviews/.pixi/envs/test-ui/lib/python3.12/site-packages/bokeh/server/contexts.py", line 317, in _cleanup_sessions
    await self._discard_session(session, should_discard_ignoring_block)
  File "/home/runner/work/geoviews/geoviews/.pixi/envs/test-ui/lib/python3.12/site-packages/bokeh/server/contexts.py", line 295, in _discard_session
    await self._application.on_session_destroyed(session_context)
  File "/home/runner/work/geoviews/geoviews/.pixi/envs/test-ui/lib/python3.12/site-packages/bokeh/application/application.py", line 246, in on_session_destroyed
    await h.on_session_destroyed(session_context)
  File "/home/runner/work/geoviews/geoviews/.pixi/envs/test-ui/lib/python3.12/site-packages/bokeh/application/handlers/lifecycle.py", line 125, in on_session_destroyed
    return self._on_session_destroyed(session_context)
  File "/home/runner/work/geoviews/geoviews/.pixi/envs/test-ui/lib/python3.12/site-packages/panel/io/application.py", line 90, in _on_session_destroyed
    callback(session_context)
  File "/home/runner/work/geoviews/geoviews/.pixi/envs/test-ui/lib/python3.12/site-packages/panel/io/application.py", line [70](https://github.com/holoviz/geoviews/actions/runs/11979575224/job/33402145460?pr=756#step:3:71), in _log_session_destroyed
    logger.info(LOG_SESSION_DESTROYED, id(doc))
Message: 'Session %s destroyed'
Arguments: (140128321335280,)

ahuang11 · 2024-11-22T20:58:38Z

:) Thanks for the assist.

Fix issues with unwrapping longitudes in RangeXY stream

8485d80

philippjfr added 2 commits October 16, 2024 16:36

Use pre-computed ranges

7769482

Small fixes

19c8d5f

ahuang11 mentioned this pull request Oct 16, 2024

Regression: RangeXY now provides incorrect longitude x_range #753

Open

ahuang11 reviewed Oct 16, 2024

View reviewed changes

geoviews/plotting/bokeh/plot.py Outdated Show resolved Hide resolved

Check for current ranges

410bd5f

ahuang11 reviewed Oct 16, 2024

View reviewed changes

geoviews/plotting/bokeh/plot.py Outdated Show resolved Hide resolved

Check in update frame too

6cea258

ahuang11 reviewed Oct 16, 2024

View reviewed changes

geoviews/plotting/bokeh/plot.py Outdated Show resolved Hide resolved

Spacing

87019c0

ahuang11 added 2 commits October 16, 2024 10:42

extend ranges from -180 to 540

e938238

rm print

02e47fa

adjust range and add comment

eea3ee9

ahuang11 requested a review from hoxbro October 24, 2024 18:25

hoxbro reviewed Oct 28, 2024

View reviewed changes

ahuang11 requested a review from hoxbro November 9, 2024 14:10

hoxbro and others added 6 commits November 9, 2024 06:11

ci: Add UI tests (#760)

9ee4e30

ci: Remove gdal pin (#761)

4536e7a

add ui tests

f7b1e7b

Merge branch 'main' into range_xy_unwrap_fix

4f20d22

Merge branch 'main' into range_xy_unwrap_fix

ff391d0

actually add the file

1a2d9b8

fix infra

b1c252a

hoxbro force-pushed the range_xy_unwrap_fix branch from fb25069 to b1c252a Compare November 22, 2024 13:57

Add cartopy download

81ba4db

hoxbro force-pushed the range_xy_unwrap_fix branch from dd72524 to 81ba4db Compare November 22, 2024 15:19

ahuang11 added 2 commits November 22, 2024 12:13

add atol

0165d78

remove irrelevant asserts

348ea2a

Add server cleanup

238513a

ahuang11 approved these changes Nov 22, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix issues with unwrapping longitudes in RangeXY stream #756

Fix issues with unwrapping longitudes in RangeXY stream #756

philippjfr commented Oct 16, 2024

codecov bot commented Oct 16, 2024 •

edited

Loading

ahuang11 commented Oct 16, 2024 •

edited

Loading

ahuang11 commented Oct 16, 2024 •

edited

Loading

ahuang11 commented Oct 16, 2024

hoxbro left a comment

ahuang11 commented Oct 28, 2024

ahuang11 commented Oct 28, 2024

hoxbro commented Oct 28, 2024

ahuang11 commented Nov 22, 2024

hoxbro commented Nov 22, 2024

hoxbro commented Nov 22, 2024 •

edited

Loading

ahuang11 commented Nov 22, 2024

ahuang11 commented Nov 22, 2024

Fix issues with unwrapping longitudes in RangeXY stream #756

Are you sure you want to change the base?

Fix issues with unwrapping longitudes in RangeXY stream #756

Conversation

philippjfr commented Oct 16, 2024

codecov bot commented Oct 16, 2024 • edited Loading

Codecov Report

ahuang11 commented Oct 16, 2024 • edited Loading

ahuang11 commented Oct 16, 2024 • edited Loading

ahuang11 commented Oct 16, 2024

hoxbro left a comment

Choose a reason for hiding this comment

ahuang11 commented Oct 28, 2024

ahuang11 commented Oct 28, 2024

hoxbro commented Oct 28, 2024

ahuang11 commented Nov 22, 2024

hoxbro commented Nov 22, 2024

hoxbro commented Nov 22, 2024 • edited Loading

ahuang11 commented Nov 22, 2024

ahuang11 commented Nov 22, 2024

codecov bot commented Oct 16, 2024 •

edited

Loading

ahuang11 commented Oct 16, 2024 •

edited

Loading

ahuang11 commented Oct 16, 2024 •

edited

Loading

hoxbro commented Nov 22, 2024 •

edited

Loading