Deltares / xugrid

Xarray and unstructured grids
https://deltares.github.io/xugrid/
MIT License
58 stars 8 forks source link

define minimal shapely version #172

Closed veenstrajelmer closed 6 months ago

veenstrajelmer commented 9 months ago

Environment with shapely<2.0.0 raises: AttributeError: module 'shapely' has no attribute 'GeometryType'. The pyproject.toml should be updated with 2.0.0 as minimal shapely version.

Edit: I see the pyproject.toml is not the issue. The issue is that it is an optional dependency. This is fine of course, but in that case upon importing it, the version check should be done, since this is not done upon installation.

ecomodeller commented 9 months ago

It seems like shapely is not really optional.

$ podman run -it --rm python:3.10-slim bash -c "pip install xugrid;python -c 'import xugrid'"

Collecting xugrid
  Downloading xugrid-0.7.1.tar.gz (134 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 134.8/134.8 kB 4.4 MB/s eta 0:00:00
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... done
...
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/usr/local/lib/python3.10/site-packages/xugrid/__init__.py", line 25, in <module>
    from xugrid.ugrid.burn import burn_vector_geometry
  File "/usr/local/lib/python3.10/site-packages/xugrid/ugrid/burn.py", line 23, in <module>
    POINT = shapely.GeometryType.POINT
  File "/usr/local/lib/python3.10/site-packages/xugrid/constants.py", line 52, in __getattr__
    raise ImportError(f"{self.name} is required for this functionality")
ImportError: shapely is required for this functionality

error.txt

Huite commented 6 months ago

Hi @ecomodeller,

Thanks for reporting this. I've moved the offending definitions into the function body with: ccc9e637f62e8f4ce7c52a1d7bd08cb8acaf724b

Huite commented 6 months ago

@veenstrajelmer: This should be handled by pip/conda/pixi and the like. I don't think it's a good idea to draw all kinds of version checking into the code.

The problem is just a classic pip problem: if you were to run pip install xugrid --all, it would install shapely 2.0. This only fails if you install xugrid via pip without all dependencies, then install another package which install an earlier shapely. Note: It's even possible some other package installs an earlier version of numpy or pandas and breaks everything entirely, but we don't want to check versions of numpy and pandas on import either.

ecomodeller commented 6 months ago
podman run -it --rm python:3.10-slim bash -c "pip install xugrid;python -c 'import xugrid'"

No errors installing from the main branch 👍

$ podman run -it --rm python:3.10-slim bash -c "pip install --upgrade pip && pip install https://github.com/Deltares/xugrid/archive/main.zip && python -c 'import xugrid'"