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

Utilisation / installer de xhydro (ou une partie) avec Pip #81

Closed
TC-FF opened this issue Feb 2, 2024 · 5 comments · Fixed by Ouranosinc/xscen#337
Closed

Utilisation / installer de xhydro (ou une partie) avec Pip #81

TC-FF opened this issue Feb 2, 2024 · 5 comments · Fixed by Ouranosinc/xscen#337
Assignees
Labels
dependencies Pull requests that update a dependency file

Comments

@TC-FF
Copy link
Collaborator

TC-FF commented Feb 2, 2024

Setup Information

Sur nos clusters de calcul, l'installation via conda est impossible (et nous ne sommes potentiellement pas les seuls)
image
xHydro est donc actuellement impossible à utiliser à cause de EMSPY. Est-ce que ESMPY pourrait être un wheel ? Ou est-ce que ça pourrait être sorti du init et que les fonctions qui nécessitent ESMPY soit dans un seul module ?

Context

No response

@TC-FF TC-FF added the dependencies Pull requests that update a dependency file label Feb 2, 2024
@Zeitsperre
Copy link
Collaborator

La complexité de ESMF est tellement large. Même quand on build la librairie en utilisant leur GitHub Action, ça prend en excès de cinq minutes (avec les configs du défaut). Le nombre des configurations nécessaires pour créer des wheels (OS * Python version * Architecture) rendre ça encore pire.

Est-ce-que l'option de rouler ESMF avec docker fonctionnerait pour vous ?

https://github.com/esmf-org/esmf?tab=readme-ov-file#pre-built-esmf

@sebastienlanglois
Copy link
Contributor

Dans la majorité des cas, nous sommes en mesure d'utiliser conda. Toutefois, pour le cas mentionné par @TC-FF, nous avons un processus qui est limité à PyPI, mais pour lequel nous pouvons exécuter du code bash en amont. On tente de voir si on peut installer ESMF par bash, mais c'est effectivement super complexe. Pour ce qui est de Docker, je ne pense pas que c'est possible pour ce cas spécifique, mais je vais regarder plus en détails.

On se questionnait aussi s'il y avait une manière de designer XHydro de manière à ce que des modules/sous-packages spécifiques puissent être appelés (ex: frequency_analysis) sans nécessairement que tous les fonctions/classes soient importées ?

@RondeauG
Copy link
Collaborator

RondeauG commented Feb 6, 2024

Désolé pour mon petit retard.

Pour l'instant, xhydro n'a pas besoin de ESMF; c'est une dépendance qui nous arrive de xscen. On est aussi en train de regarder ça de notre bord pour les mêmes raisons (environnement conda pas toujours possible sur les clusters de calcul).

Une solution possible serait quelque chose similaire à ce qui est fait sur xdatasets: https://github.com/hydrologie/xdatasets/blob/master/xdatasets/spatial.py

try:
    import xagg as xa
except ImportError:
    warnings.warn("xagg is not installed. Please install it with `pip install xagg`")
    xa = None

Mais il faudrait que ça soit dans xscen, pas xhydro. Je vais en discuter de notre côté.

@RondeauG
Copy link
Collaborator

RondeauG commented Feb 6, 2024

On se questionnait aussi s'il y avait une manière de designer XHydro de manière à ce que des modules/sous-packages spécifiques puissent être appelés (ex: frequency_analysis) sans nécessairement que tous les fonctions/classes soient importées ?

On peut se servir du __init__ pour décider ce qui est importé quand on fait import xhydro ou import xhydro.frequency_analysis, mais dans ce cas-ci ça n'aiderait pas parce que xscen est importé dans les deux cas.

aulemahal added a commit to Ouranosinc/xscen that referenced this issue Feb 7, 2024
<!-- Please ensure the PR fulfills the following requirements! -->
<!-- If this is your first PR, make sure to add your details to the
AUTHORS.rst! -->
### Pull Request Checklist:
- [x] This PR addresses an already opened issue (for bug fixes /
features)
    - This PR fixes hydrologie/xhydro#81
- [ ] (If applicable) Documentation has been added / updated (for bug
fixes / features).
- [x] (If applicable) Tests have been updated.
- [x] This PR does not seem to break the templates.
- [x] CHANGES.rst has been updated (with summary of main changes).
- [x] Link to issue (:issue:`number`) and pull request (:pull:`number`)
has been added.

### What kind of change does this PR introduce?

* Make xesmf optional:
+ Remove it from the dependencies in "pyproject.toml". It is now an
"extra" dependency. I did not remove it from the environment.yml files
as the issue doesn't occur with conda.
+ Add a few "try: import" throughout the package and raise on call time
when xesmf is not available.
    + Skip the tests that require xesmf, if not present.
+ Edit the tox config to remove any usage of conda, since it is now not
needed.

### Does this PR introduce a breaking change?
No.

### Other information:
@Zeitsperre I may have been heavy handed in editing the workflow and tox
configuration. My idea was that the "test-pypi" didn't need xesmf/esmf
anymore and so I removed any mention of it and of mamba/conda. So it can
really be a "test-pypi" ci.

We accept the decrease in coveralls for the Pypi tests, it is a consequence of removing xesmf.
@RondeauG
Copy link
Collaborator

Réglé avec xscen 0.8.2, qui est maintenant disponible sur PyPI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants