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

Uniformize environments and update dependencies #140

Merged
merged 3 commits into from
Jan 17, 2023
Merged

Uniformize environments and update dependencies #140

merged 3 commits into from
Jan 17, 2023

Conversation

aulemahal
Copy link
Collaborator

Pull Request Checklist:

  • This PR addresses an already opened issue (for bug fixes / features)
    • This PR fixes #xyz
  • (If applicable) Documentation has been added / updated (for bug fixes / features)
  • HISTORY.rst has been updated (with summary of main changes)
    • Link to issue (:issue:number) and pull request (:pull:number) has been added

What kind of change does this PR introduce?

  • Reorder the environment files to fit what's in setup.py (an alphabetical order)
  • Remove fixes for esmpy 8.4, as xESMF 0.7 takes care of that (the pin is bumped).
  • Add a (brand new!) fix for esmpy 8.4 to avoid installing packages with pip with running pip install -e . on xscen
  • Remove numba for the envs, it is not a direct dep and the bug related to version 0.55 is caduc with 0.56.
  • Uniformize the two envs : h5netcdf, pyarrow, fsspec

Does this PR introduce a breaking change?

No.

Other information:

This still install netCDF4 1.6.2, I'm not sure if this version has been fixed or not?

@aulemahal
Copy link
Collaborator Author

Well...
It works on my machine though...

@Zeitsperre
Copy link
Contributor

Looks like an issue with ESMF?

@aulemahal
Copy link
Collaborator Author

Oh I see, I thought the test failures were because of my PR, but your PRs to main fix them, nice!

The RTD error seems to be related to this issue : conda-forge/esmf-feedstock#91 and the solution is to explicitly activate the env, which I don't think RTD does...

@aulemahal
Copy link
Collaborator Author

Related RTD issue is : readthedocs/readthedocs.org#4067. We will need to patch this environment variable in the code, I'll push something.

@Zeitsperre
Copy link
Contributor

I'm just curious, but is there any way for us to manually restart the environment using the .readthedocs.yml? I.e.:

build:
  jobs:
    pre-build:
      - conda deactivate
      - conda activate xscen-env

@aulemahal
Copy link
Collaborator Author

aulemahal commented Jan 17, 2023

Well, I thought of that and decided not to do it for 2 reasons:

  • I didn't want to interfere with RTD's machinery.
  • Can you call conda activate if you haven't called conda init ? Does RTD call the latter? I'm suspecting it doesn't and this is why the conventional activation mechanism isn't used.

@Zeitsperre
Copy link
Contributor

That makes sense. Was just wondering. Your fix is clear enough for me. 👍

@aulemahal aulemahal merged commit a1f3957 into main Jan 17, 2023
@aulemahal aulemahal deleted the fix-envs branch January 17, 2023 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants