Skip to content

MAINT install of pinned vers for travis #13281

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

Closed
wants to merge 1 commit into from

Conversation

NelleV
Copy link
Member

@NelleV NelleV commented Jan 24, 2019

Pinned version of dependencies need to be installed after the rest of
the packages. This will downgrade them. Else, sometimes, pip tries to installs
the wrong version of the package, and fails.

Right now, this isn't a problem on master, but this should probably still be ported forward to master. Happy to create a PR.

This is due to a pip release

Pinned version of dependencies need to be installed *after* the rest of
the packages. This will downgrade them. Else, sometimes, pip tries to installs
the wrong version of the package, and fails.
@timhoffm
Copy link
Member

Shouldn't this go to vthe 3.0.2-doc to fix #13274?

@NelleV
Copy link
Member Author

NelleV commented Jan 25, 2019

No clue. I'm super confused on why we have a documentation branch that moves forwards after the code is released. This is also not a documentation fix.
I guess we can backport this to 3.0.2-doc with meeseekv. I'd rather not create a manual patch, as I believe that merging patches on an already release code base is bad practice.

@QuLogic
Copy link
Member

QuLogic commented Jan 25, 2019

I still think we should apply something like #12398 and not run full tests for doc branches.

@NelleV
Copy link
Member Author

NelleV commented Jan 25, 2019

This PR still needs to get merged. It fixes something that is unrelated to our docs.

@anntzer
Copy link
Contributor

anntzer commented Jan 25, 2019

I... don't think the interpretation of the problem is correct?

Right now (since #12878) the 3.0.2-doc branch has the following travis_all.txt:

<elided>
# pytest-timeout master depends on pytest>=3.6. Testing with pytest 3.4 is
# still supported; this is tested by the first travis python 3.5 build
# pytest>=3.8 throws RemovedInPytest4Warning (github #12825)
pytest>=3.6,<3.8
pytest-cov
pytest-faulthandler
pytest-rerunfailures
pytest-timeout
pytest-xdist
<elided>

and the following travis35.txt (constraints file):

<elided>
pytest==3.4
pytest-cov==2.3.1
pytest-timeout==1.2.1  # Newer pytest-timeouts don't support pytest 3.4.
pytest-rerunfailures<5  # newer versions require pytest3.6
<elided>

so we're giving pip contradictory constraints on pytest. I guess it would be nicer if pip signalled this properly, but I think(?) the correct fix on our side is to make the constraints not contradictory anymore.

@NelleV
Copy link
Member Author

NelleV commented Jan 26, 2019

@anntzer The new version is fine, as it installs all the dependencies in the first file, and then installs the pinned version from the second file. Sure, it installs and then downgrades pytests, but it works, and the logic is not flawed.

Would unpinning the version in travis_all.txt work? No, because the new version of pip doesn't care about the order of the requirement files nor whether one version contains a pinned version or the other doesn't. In particular, in our last failed build on 3.0.x, pytest-xdist fails to be installed because it is attempting to install the newer unpinned version from travis_all.txt instead of the pinned version of travis35.txt

Is it the best way to do this? No, it is not, as it installs some of the packages twice. But we have too many dependencies to be able to have a clean setup that doesn't require maintaining entirely separate requirements files for all of the versions we are testing.

@anntzer
Copy link
Contributor

anntzer commented Jan 26, 2019

Would #12398 not fix the problem much more simply by just not running the test suite on the doc branch?

@NelleV
Copy link
Member Author

NelleV commented Jan 26, 2019

This patch is not on the documentation branch.

@anntzer
Copy link
Contributor

anntzer commented Jan 26, 2019

I am extremely confused now. Isn't the 3.0.x branch just fine? The last 5 builds are green on https://travis-ci.org/matplotlib/matplotlib/branches.

@tacaswell tacaswell added this to the v3.0.3 milestone Jan 27, 2019
@NelleV
Copy link
Member Author

NelleV commented Jan 28, 2019

@anntzer I'll double check that it works on the doc branch once I'm back from Australia.

@NelleV NelleV closed this Feb 2, 2019
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.

5 participants