search

earthobservations / wetterdienst

Open weather data for humans.
https://wetterdienst.readthedocs.io/
MIT License
349 stars 54 forks source link

wradlib: Recently released wradlib 1.19.0 has an installation flaw? #876

Closed amotl closed 1 year ago

amotl commented 1 year ago

Observation

At GH-874, we discovered this on a CI run which accidentally installed wradlib 1.19.0.

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/usr/local/lib/python3.11/dist-packages/wradlib/__init__.py", line 24, in <module>
    from . import adjust  # noqa
    ^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/wradlib/adjust.py", line 118, in <module>
    from wradlib import ipol, util
  File "/usr/local/lib/python3.11/dist-packages/wradlib/ipol.py", line 52, in <module>
    from wradlib import georef, util, zonalstats
  File "/usr/local/lib/python3.11/dist-packages/wradlib/zonalstats.py", line 66, in <module>
    from wradlib import georef, io
  File "/usr/local/lib/python3.11/dist-packages/wradlib/io/__init__.py", line 28, in <module>
    from .backends import *
  File "/usr/local/lib/python3.11/dist-packages/wradlib/io/backends.py", line 49, in <module>
    from xradar.io.backends import CfRadial1BackendEntrypoint as XCfRadial1BackendEntrypoint
ModuleNotFoundError: No module named 'xradar'

-- https://github.com/earthobservations/wetterdienst/actions/runs/4193227334/jobs/7269833277#step:7:1136

/cc @kmuehlbauer

kmuehlbauer commented 1 year ago

In 1.19 xradar has been added as dependency.

It was added in requirements.txt, I can't immediately see, why this brakes here.

amotl commented 1 year ago

It may be because the pinning of xradar>=0.0.13 is insufficient for an alpha library. It will probably break too quickly, so maybe use exact version pinning instead?

Remark: I don't want to hamper your velocity, just sharing my thoughts. Kudos for conceiving xradar by the way - I've taken a few glimpses and it looks excellent.

amotl commented 1 year ago

Ah, forget about my suggestion, you've probably synchronised things well, because 0.0.13 is actually the latest release. Sorry for the noise, I will try to come up with a minimal repro if I can find some minutes.

kmuehlbauer commented 1 year ago

The reason

RUN python -m pip install --no-cache-dir --prefer-binary --no-deps wradlib

--no-deps will miss installing xradar here

xradar is now at 0.0.13, so latest version. Most of the code I've ported verbatim from wradlib to xradar and it is as stable as it gets. I've made sure that wradlib is only affected in the slightest way.

wradlib currently only uses the backend code from xradar, which is not about to change, besides bug fixes. I hope this is a little comforting with regard to the beta status of xradar.

amotl commented 1 year ago

--no-deps will miss installing xradar here

Oh, thank you so much for spotting this. I missed it. I actually don't know why we have those lines here.

https://github.com/earthobservations/wetterdienst/blob/40b58000915a34d6b875c9ed4c13bff51831dd82/.github/release/full/Dockerfile#L16-L17

https://github.com/earthobservations/wetterdienst/blob/40b58000915a34d6b875c9ed4c13bff51831dd82/.github/workflows/install.sh#L18

It would be better to install wradlib as a regular optional dependency of wetterdienst there, see GH-875.

amotl commented 1 year ago

It would be better to install wradlib as a regular optional dependency of wetterdienst there, see https://github.com/earthobservations/wetterdienst/issues/875.

I gave it a try with GH-878, but immediately got tripped, and by that, reminded, about the error we are observing then, because we don't use Anaconda on our CI, and the dependency chain wradlib -> Cartopy -> GDAL does not resolve well [^1].

[^1]: I am specifically wondering why wradlib pulls in Cartopy, even though it is only listed as optional dependency, right?

amotl commented 1 year ago

We've improved the situation with GH-879. Thanks again!