search

ecmwf / ecmwflibs

Apache License 2.0
15 stars 9 forks source link

ecmwflibs prevents eccodes-python to use locally built libeccodes.so #5

Closed thebaptiste closed 2 years ago

thebaptiste commented 2 years ago

If ecwmflibs is available in PYTHONPATH, eccodes-python will not use a locally built libeccodes.so but the one provided by ecmwlibs. It will change the behavior of eccodes-python (see https://github.com/ecmwf/eccodes-python/issues/61), especially since the ecwmflibs eccodes library is built with ENABLE_MEMFS=1

This can be changed setting ECMWFLIBS_ECCODES to the locally built libeccodes.so but it is not very intuitive (and I think not documented ?).

May be ecmwflibs should first try to find eccodes and MagPlus locally (if they are provided by LD_LIBRARY_PATH), and only if they are not found use the ones provided by ecmwflibs ?

Best regards

floriankrb commented 2 years ago

The purpose of ecmwflibs is to provide ECMWF libraries that works out-of-the-box. Tweaking its behaviour using env variables does get along this principle. I would say that If you do not want to use the lib provided by ecmwflibs, then don't install ecmwflibs, or recompile your own version.

Reading https://github.com/ecmwf/eccodes-python/issues/61, I understand that installing ecmwflibs has an impact on eccodes-python, this choice (of giving higher priority to ecmwflibs) is made within eccodes-python and is a eccodes-python issue. I will answer there.

thebaptiste commented 2 years ago

If I install ecmwflibs, it is not "for the fun" or "looking for trouble" (I would prefer not to), it's because it's a required dependency of an other package we are using in the same environment (climetlab). May be we will have our own version of ecmwflibs... not providing any library !

I understand the purpose of ecmwflibs. But having a python package (ecmwflibs), which is not a dependency of another one (eccodes-python) and that change, when installed, the behaviour of this package, is quite surprising for the user...

thebaptiste commented 2 years ago

Perhaps ecmwflibs should not be a required dependency of any ECMWF python package. In this case, it would not be a problem to give priority to ecmwflibs. Packages such as eccodes-python or climetlab will try to use it, if they don't find ecmwflibs they would try to find the required libraries locally (that's what eccodes-python do), and if they can't find them they would fail with a message such as "Library eccodes not found. Please install ecmwflibs or provide a path to library eccodes on your system". In this case, installing ecmwflibs or not would be a user's choice.

thebaptiste commented 2 years ago

May be with optional dependency climetlab ==> install climetlab with default dependencies, without optional dependency ecmwflibs climetlab[ecmwflibs] ==> install climetlab with default dependencies + optional dependency ecmwflibs

thebaptiste commented 2 years ago

And sorry to bother ECMWF staff with all my comments. May be you will find them useful, may be not. If you don't change anything it's not a problem for me, I have a workaround. Regards

floriankrb commented 2 years ago

No problem, it is nice to have feedback.

I think we should keep the simple pip install climetlab to install all what you need: the default installation should be able to open grib files a show some plots, therefore the dependency on ecmwflibs. Currently, climetlab makes assumption about which compilation flags where use, and using a local install of eccodes (i.e. not ecmwflibs), it may work but it is not tested on github actions.

Nevertheless, I agree that the packages climetlab, ecmwflibs, eccodes-python should not break each other.

floriankrb commented 2 years ago

The commit https://github.com/ecmwf/ecmwflibs/commit/f97bc933e4f952670d1db60ffbe6fb92b5f93234 solves the problem that "this is not intuitive". Adding warning and help when env variables are ignored.

thebaptiste commented 2 years ago

Ok. That looks fine to me. Thanks.