Skip to content

CI: Install pytest>=3.4 in Travis #2659

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

Merged
merged 3 commits into from
Jul 27, 2018
Merged

CI: Install pytest>=3.4 in Travis #2659

merged 3 commits into from
Jul 27, 2018

Conversation

effigies
Copy link
Member

Summary

pytest-xdist 1.22.4 bumped their requirement to pytest >=3.4 (ours is >=3.0). It's unclear why this is failing to resolve. Creating this issue to see if there's a conflict.

See Travis errors in #2658.

Possibly related: #2649

Acknowledgment

  • (Mandatory) I acknowledge that this contribution will be available under the Apache 2 license.

@djarecka
Copy link
Collaborator

it looks like they just had a new release. If you have problem, we can temporary change to 1.22.3, it was working

@effigies
Copy link
Member Author

That might be a better solution. Hopefully it's just a temporary resolution error and we don't actually have any conflicts that prevent us from using >=3.4, since the current pytest series is 3.6.x.

@codecov-io
Copy link

codecov-io commented Jul 27, 2018

Codecov Report

Merging #2659 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2659      +/-   ##
==========================================
- Coverage   67.62%   67.61%   -0.01%     
==========================================
  Files         340      340              
  Lines       43054    43054              
  Branches     5329     5329              
==========================================
- Hits        29115    29113       -2     
- Misses      13237    13240       +3     
+ Partials      702      701       -1
Flag Coverage Δ
#smoketests 50.51% <100%> (ø) ⬆️
#unittests 65.08% <100%> (-0.01%) ⬇️
Impacted Files Coverage Δ
nipype/info.py 94.02% <100%> (ø) ⬆️
nipype/interfaces/dipy/setup.py 0% <0%> (-25%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aeb0874...0884e27. Read the comment docs.

@djarecka
Copy link
Collaborator

@effigies - I'll merge it to #2637, so the tests can run

@djarecka
Copy link
Collaborator

or can we just merge to master? looks like everyone is happy with pytest>3.4

@effigies
Copy link
Member Author

Yeah, updating pip wasn't sufficient to get pip to correctly resolve the dependency. The only alternative I see is to add a pip install --upgrade pytest>=3.4 to Travis.

@satra @oesteban Any thoughts on the best way to proceed? 3.4 is from this year, so this is a fairly strong constraint for people who don't need the latest pytest-xdist.

@djarecka
Copy link
Collaborator

@effigies - not sure why this is a problem. If they already have old pytest-xdist the older pytest will work. If they somehow got pytest-xdist that was released today morning, they should be ok with getting pytest from Jan.

@effigies
Copy link
Member Author

Sure. But now we're requiring pytest 3.4. In general, it's good to be as flexible as possible to avoid putting users in a place where they have conflicting requirements. For instance, suppose somebody has a package that requires pytest<=3.3, and they have some pytest-xdist that is compatible with pytest==3.3. Now they want to install nipype==1.1.1, and suddenly they have an unresolvable dependency conflict. We don't actually need pytest 3.4, but the latest version of one of our dependencies causes tests to break on Travis, so we update it... It's not ideal.

@djarecka
Copy link
Collaborator

@effigies - so one option would be to remove xdist, i.e. pytest -n from nipype.test() and keep it only for "test" installation, for people who want to test it regularly

@effigies
Copy link
Member Author

Eh. I decided to go the Travis hack route. It really is just a quirk of Travis' default python environment and a pip bug. Keeping a reasonable set of dependencies shouldn't affect typical users. If I'm wrong, and we need to update the dependency in the future, we can.

@effigies effigies changed the title DEP: Update pytest dependency CI: Install pytest>=3.4 in Travis Jul 27, 2018
@effigies effigies added this to the 1.1.1 milestone Jul 27, 2018
@djarecka
Copy link
Collaborator

@effigies - but the standard pip installation of nipype will not lead to the error for nipype.test()?

@effigies
Copy link
Member Author

The case in which it does is if you have pytest>=3.0,<3.4 already installed, but not pytest-xdist.

Sure there might be some setups like that, and we can walk people through some solution that fits their needs. Likely pip install --upgrade pytest is sufficient, but at least we won't be imposing an unresolvable dependency conflict.

@djarecka
Copy link
Collaborator

ok, we can wait and see if anyone complains

@effigies effigies merged commit a176815 into nipy:master Jul 27, 2018
@effigies effigies deleted the dep/pytest branch July 27, 2018 18:36
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.

3 participants