Skip to content

Limit pytest version for LTS#8173

Merged
bsipocz merged 2 commits intoastropy:v2.0.xfrom
bsipocz:pytest_limit_LTS_requirement
Nov 24, 2018
Merged

Limit pytest version for LTS#8173
bsipocz merged 2 commits intoastropy:v2.0.xfrom
bsipocz:pytest_limit_LTS_requirement

Conversation

@bsipocz
Copy link
Copy Markdown
Member

@bsipocz bsipocz commented Nov 23, 2018

Pytest 4.0 is not compatible with LTS as we can't really do much about it as we cannot remove API from this branch, so the workaround has to be to add an upper limit to the version dependency.

See: #7782 (comment)

Copy link
Copy Markdown
Contributor

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

I agree this seems like the appropriate solution! 😁

Copy link
Copy Markdown
Member

@pllim pllim left a comment

Choose a reason for hiding this comment

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

LGTM (as long as the tests can be made to pass).

@pllim pllim added the zzz 💤 merge-when-ci-passes Do not use: We have auto-merge option now. label Nov 24, 2018
@bsipocz
Copy link
Copy Markdown
Member Author

bsipocz commented Nov 24, 2018

Re passing tests: this branch has a circleCI issue for a long time on 32bit that we don't understand (but it's somewhat transient, and doesn't seem to be astropy related).

But the rest of the failures is due to astropy/ci-helpers#337, so I think I would just pin pytest version for those to make them pass (pip install perfectly takes the version dependency into account, so for the users it shouldn't be such a big deal?)

@bsipocz bsipocz force-pushed the pytest_limit_LTS_requirement branch from 0594640 to 6fd3930 Compare November 24, 2018 05:50
@bsipocz
Copy link
Copy Markdown
Member Author

bsipocz commented Nov 24, 2018

All but the usual 32bit is passing, so I merge this now.

@bsipocz bsipocz merged commit a68f969 into astropy:v2.0.x Nov 24, 2018
@olebole
Copy link
Copy Markdown
Member

olebole commented Nov 25, 2018

When Debian updates its pytest to 4 (currently, it is 3.10), I will have the choice to either disable testing for the 2.x legacy code, or to remove it completely (... and so almost the whole Python 2 astronomy ecosystem).

BTW, Is there a reason why it limits to <3.10 and not <4.0? The comments suggest that 3.10 should be supportable, right?

@bsipocz
Copy link
Copy Markdown
Member Author

bsipocz commented Nov 25, 2018

pytest 3.10 also shows the issue: https://travis-ci.org/astropy/astropy/jobs/459029569#L4185, I suppose I should have done it more consistently and specify it in setup.cfg, too.

And yes, maintaining the 2.0.x LTS branch is starting to be rather painful for us, too but we can't do much about it if we intend to keep it a no API changing one.
But it's still self consistent with its bundled libraries and dependencies listed in setup.py. If distributions chooses to unbundle/ignore those, I suppose they can stop supporting the LTS whenever it gets too many version clashes? Users can still choose not to update their distros, or switch to use conda/pip?

@nicoddemus
Copy link
Copy Markdown
Contributor

nicoddemus commented Nov 26, 2018

And following @bsipocz, just want to say that us from pytest's side hate to break things on user's side like this, but sometimes this kind of breakage is necessary for us to move forward.

@bsipocz
Copy link
Copy Markdown
Member Author

bsipocz commented Nov 26, 2018

Re @olebole - It seems that the pytest collection is broken for >=3.7 & <4.0, so it seems we need to limit LTS to pytest <3.7.
It seems we can't really do much about it, so settled on listing it in known issues.

Details: #8177

@olebole
Copy link
Copy Markdown
Member

olebole commented Nov 27, 2018

@nicoddemus since a number of packages seem to fail in Python 2 with pytest 3.10 (my experience in Debian; I could provide a list if needed): would it be possible to still have 3.X version that fixes the problems? That would allow me to keep the Python 2 Astropy ecosystem in the next Debian release.
I am however also fine with removing it.

@nicoddemus
Copy link
Copy Markdown
Contributor

@olebole I would love to help here, but we don't have the capability in pytest to have a 3.X version now... our versioning system does not accommodate that I'm afraid. ☹️

@olebole
Copy link
Copy Markdown
Member

olebole commented Nov 27, 2018

@nicoddemus OK, thanks for clarification. Our next release (Buster) will be out not before summer/fall 2019, so it is perfectly reasonable to drop Python 2 support for Astropy.

@bsipocz
Copy link
Copy Markdown
Member Author

bsipocz commented Nov 27, 2018

And we will stop supporting python2 ~Novemberish next year, so this all comes together nicely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

installation testing zzz 💤 merge-when-ci-passes Do not use: We have auto-merge option now.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants