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

Requirements re-haul #3812

Merged
merged 6 commits into from
Sep 8, 2020
Merged

Conversation

bjlittle
Copy link
Member

@bjlittle bjlittle commented Sep 3, 2020

This PR rationalises the dependencies of iris for using conda with the CI travis-ci and readthedocs services, and also for PyPI.

Our current custom approach to manage both conda and PyPI dependencies as one, in general is not intuitive, nor standard, and leads to confusion for both developers and users alike.

Rather, this PR adopts standard community known approaches using self-contained conda environment yml files, and setup.py to manage PyPI dependencies for iris.

All the dependencies now live under the root directory requirements:

requirements
|-- all.txt
|-- ci
|   |-- iris.yml -> py37.yml
|   |-- py36.yml
|   |-- py37.yml
|   `-- readthedocs.yml -> iris.yml
|-- core.txt
|-- docs.txt
|-- setup.txt
`-- test.txt

For conda

As a developer, configuring iris with all the relevant dependencies that you need to code, test, and build the docs is now as simple as:

> conda env create --file=requirements/ci/iris.yml
> . activate iris-dev

Using conda over pip is preferred, due to the superior package dependency management and package coverage. Note that, for conda all packages are pulled from conda-forge and the associated yml files will create the named environment called iris-dev.

So it's one environment for all when it comes to coding, testing, and documentation (although now we can differentiate package content and package versions based on the Python version) which ensures (as far a possible without freezing the environment) that there is environment stability no matter what you are doing i.e., there are no package or package version differences for travis-ci, readthedocs, coding, local testing or building the docs.

The minor downside is that there is some duplication, but IMHO the simplicity and utility of this approach far outweighs the disadvantages of our current custom mechanisms to cherry-pick and manage package dependencies.

Note that, the readthedocs.yml is an alias for the iris.yml file, which is an alias for py37.yml file. As we roll forwards with testing iris with more modern versions of python, we simple create those Python version specific yml files e.g., py38.yml, re-point iris.yml to py38.py, and update .travis-ci.yml appropriately to update the test matrix.

For pip

It's not possible to only use pip to install scitools-iris (PyPI name) due to all the dependencies not being available on PyPI e.g., proj, udunits2 etc.

However, we now host pyke as scitools-pyke on PyPI (but the hope is to purge pyke as technical debt ASAP 🙏).

This PR introduces the pyproject.toml file to support PEP517 and PEP518 building during installation, thus allowing the pyke rules for iris to be built when using pip - this wasn't possible before.

Note that, all the requirements/*.txt files in conjunction with the setup.py file service installation of scitools-iris and it's extras, namely [all], [docs], and [test], pretty much as before.

@bjlittle bjlittle added this to the v3.0.0 milestone Sep 3, 2020
"all": pip_requirements("all"),
"extensions": pip_requirements("extensions"),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dropping the use of extensions as cartopy will now pull in gdal, and iris-grib is not used now with the conda requirements, so mirroring that here... hence, no extensions.

@@ -4,7 +4,7 @@ build:
image: latest

conda:
environment: ci/requirements/readthedocs.yml
environment: requirements/ci/readthedocs.yml
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tkknight This should be enough for readthedocs to find the associated yml file.

@pp-mo
Copy link
Member

pp-mo commented Sep 4, 2020

This is awesome + long overdue.
Right off the top of the head, a few general points arising for me...

  • as the account here is so good, can we document it somehow ?
  • we need to be clearer about the categories.
    • core/setup/test/docs are all good,
    • "extensions" was always peculiar.
    • But what is "all" for ??
  • do we really have to have multiple files for different python versions??
    Yuk, frankly 🤢

@bjlittle
Copy link
Member Author

bjlittle commented Sep 4, 2020

@pp-mo Yeah, I've been meaning to do this for a long, long time... but just never got around to it, but glad I finally did 🎉

I agree with everything in your comment above 👍

This definitely should be documented and it will. I've left this for @tkknight to do as a follow up task, as he's totally swarming all over the documentation (mucho ❤️), and this PR unblocks him. I suspect that my description detail above may serve as a strawman to some developer notes in the follow-up PR.

Also, yeah, having the py36.yml and py37.yml is a slight pimple on the princess, but other projects do also take this approach e.g., see here.

To be honest, I'm willing to live with this and it does give us the scope to have dependencies per Python version, and we can maintain those dependencies historically going forwards, which wasn't possible before... so there are benefits, and at the end of the day I think I can see past the 🤢 if you can?

@tkknight
Copy link
Contributor

tkknight commented Sep 5, 2020

Hey @bjlittle. Here is the PR (#3817) to bring the documentation inline with this PR. It will be in draft until this PR is merged.

@bjlittle
Copy link
Member Author

bjlittle commented Sep 6, 2020

@pp-mo Also, I'm keen that we use conda env create rather than conda create, as it's opinionated in terms of naming the environment and the channel that it pulls packages from, plus it's a very simple instruction to the user/developer, see #3817... with no scope for if'ing or but'ing about environment names, conda channels or Python versions.

@tkknight
Copy link
Contributor

tkknight commented Sep 7, 2020

I'm happy to merge this now. Any last comments @pp-mo ?

@pp-mo
Copy link
Member

pp-mo commented Sep 7, 2020

@tkknight I'm happy to merge this now. Any last comments @pp-mo ?

No, after some thought I'm 👍

@tkknight tkknight merged commit 4fab30a into SciTools:master Sep 8, 2020
tkknight added a commit to tkknight/iris that referenced this pull request Sep 8, 2020
* master:
  Add template collapsible markdown section (SciTools#3823)
  Requirements re-haul (SciTools#3812)
  Automate pull request labels (SciTools#3819)
  move whatsnew contributions (SciTools#3816)
  Parse with packaging version (SciTools#3815)
  Add "What's New" entries for unpinning Cartopy, Matplotlib, Proj (SciTools#3811)
  tidy issue templates (SciTools#3814)
  Legacy doc notice (SciTools#3813)
tkknight added a commit to tkknight/iris that referenced this pull request Sep 8, 2020
* master:
  Updated Iris install instructions (SciTools#3817)
  Delete labeler.yml
  Delete label.yml
  Create label.yml
  Delete labeler.yml
  Update labeler.yml
  Update labeler.yml
  Add template collapsible markdown section (SciTools#3823)
  Requirements re-haul (SciTools#3812)
  Automate pull request labels (SciTools#3819)
@bjlittle bjlittle deleted the requirements-rehaul branch September 8, 2020 14:53
tkknight added a commit to tkknight/iris that referenced this pull request Sep 11, 2020
* master: (36 commits)
  Rework whatsnew into new scheme. (SciTools#3834)
  Lazy regridding with Linear, Nearest, and AreaWeighted (SciTools#3701)
  Iris readme minimal (SciTools#3833)
  Fix sphinx logger (SciTools#3832)
  whatsnew entry for dependencies (SciTools#3831)
  Added link to scitools.org.uk on the main index. (SciTools#3830)
  Updated Iris install instructions (SciTools#3817)
  Delete labeler.yml
  Delete label.yml
  Create label.yml
  Delete labeler.yml
  Update labeler.yml
  Update labeler.yml
  Add template collapsible markdown section (SciTools#3823)
  Requirements re-haul (SciTools#3812)
  Automate pull request labels (SciTools#3819)
  move whatsnew contributions (SciTools#3816)
  Parse with packaging version (SciTools#3815)
  Add "What's New" entries for unpinning Cartopy, Matplotlib, Proj (SciTools#3811)
  tidy issue templates (SciTools#3814)
  ...
trexfeathers added a commit to trexfeathers/iris-grib that referenced this pull request Sep 30, 2020
trexfeathers added a commit to trexfeathers/iris-grib that referenced this pull request Sep 30, 2020
pp-mo pushed a commit to pp-mo/iris-grib that referenced this pull request Sep 30, 2020
pp-mo pushed a commit to pp-mo/iris-grib that referenced this pull request Sep 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants