Fixes #53: Avoid Iris 3.2.0.post0 #54
Conversation
|
Honestly, I'm very confused about the Travis builds. I have checked and Travis-CI finds master, the black+isort branch, and the 3d averages branch – but not this one? (see screenshot) I have tried whether the pattern of the branch name is a problem but it didn't lead to any problems on my fork. I'll have to check out later if this problem persists with every new branch we're creating here of if it's just this one that's causing problems.
|
.travis.yml
Outdated
| @@ -30,7 +30,7 @@ install: | |||
| - conda activate test-environment | |||
| - conda env update -n test-environment --quiet --file conda_environment.yml | |||
| - conda install pip | |||
| - conda install -c conda-forge iris | |||
| - conda install -c conda-forge "iris>=3.1,!=3.2.0.post0" | |||
There was a problem hiding this comment.
we can actually just delete this line. line 31 already installs iris from the conda_environment file.
There was a problem hiding this comment.
we can actually just delete this line. line 31 already installs iris from the conda_environment file.
Okay, I was wondering about this.
BUT: I think the conda env update command comes with it's own issues. At least at some point, it upgraded my Python version (because it says python>=3.6), which is not what we want, do we? So I wonder in general how reliable this recipe really is.
There was a problem hiding this comment.
For reference, this is what I found is a reliable way to install the test environment:
conda create -n testenv -c conda-forge python=3.x
conda activate testenv
conda install -c conda-forge "iris>=3.1,!=3.2.0.post0"
pip install -e . # source install from the cloned git repo
pip install [...] # whatever else is needed to run the tests (coverage, pytest, ...)
This can of course be adapted again to some of the helpful extras of the .travis recipe and also using some environment file.
There was a problem hiding this comment.
I mean, we can also remove the python>=3.6 since a too low Python version will lead to a failed installation anyhow. The problem with having it in .travis.yml is simply that we have to update this in three places every time we change something about the dependencies (setup.py, conda_environment.yml, .travis.yml)
conda_environment.yml
Outdated
| - iris>=3.1,!=3.2.0.post0 |
There was a problem hiding this comment.
I don't really understand the decision for this yet: In #53 you mention that the actual errors leave when you install Iris version 3.1. Why not change this to iris==3.1 or iris>=3.1,<=3.2, then?
There was a problem hiding this comment.
Especially if Iris drops support for Python 3.7 from now on, I think we should start thinking if we just keep it at the current version or require 3.8 or higher.
There was a problem hiding this comment.
I don't really understand the decision for this yet: In #53 you mention that the actual errors leave when you install Iris version 3.1. Why not change this to
iris==3.1oriris>=3.1,<=3.2, then?
We know that Iris 3.2.0.post0 is buggy, so a specific specification would be to exclude that. We do not want to exclude versions that are not known to be buggy. Specifically, we want to allow for newer Iris versions (which presumably have the bug fixed).
Also borrowing from the discussion in ESMValGroup/ESMValCore#1509.
There was a problem hiding this comment.
But still, the tests are failing with Iris 3.2 (without the .post0 prefix). So maybe use !=3.2 instead of !=3.2.0.post0?
There was a problem hiding this comment.
Alright, I see that there are actually two buggy package candidates on conda-forge, I haven't seen this earlier (and it was working for me).
So !=3.2.0.post0 is not enough, but what about !=3.2.0, which is relaxing the excluding rule as little as possible? It is also what the ESMValTool people seem to agree on.
There was a problem hiding this comment.
... should have read the whole discussion over there. Both !=3.2.0.post0 and !=3.2.0 need to be pinned in that case.
There was a problem hiding this comment.
Yes, that sounds a lot better! Also sounds pretty much like what has been discussed in the other thread :)
| cube = iris.load_cube(src, var).collapsed("latitude", iris.analysis.MEAN) | ||
| # print(cube) # Workaround for the Iris bug | ||
| iris.save(cube, str(dst)) | ||
|
|
There was a problem hiding this comment.
nice to add a test for this directly! 👍
Okay, at least it is not me being stupid! |
|
Just to make sure I understand: Is your conclusion also that Travis is not triggered to run the tests for this branch? |
Exactly! Seems not to get triggered, I have no idea why |
|
@uwefladrich now Travis found the branch 😄 |
No description provided.