Skip to content

env: Add surrogate for pytest.deprecated_call for ptyest<3.9 #2923

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
Apr 2, 2021

Conversation

EricCousineau-TRI
Copy link
Collaborator

Description

Resolves #2922

Suggested changelog entry:

Testing only; doesn't merit a change?

@EricCousineau-TRI
Copy link
Collaborator Author

@YannickJadoul or @rwgk Any chance y'all have time for a quick stamp?

@EricCousineau-TRI
Copy link
Collaborator Author

I'd also be fine with telling pybind11 to fail fast if an old version of pytest is found (e.g. require >=3.9).

Lemme know!

@rwgk
Copy link
Collaborator

rwgk commented Mar 29, 2021

I'd also be fine with telling pybind11 to fail fast if an old version of pytest is found (e.g. require >=3.9).

Lemme know!

I just looked, pytest 3.9 was released in October 2018, nearly 2.5 years ago.
My vote: require >=3.9
In the interest of simplicity/clarity/long-term maintenance.

@EricCousineau-TRI
Copy link
Collaborator Author

Er, only caveat is that Ubuntu 18.04 also provisions pytest==3.3.2 for CPython 3.6.9 :(

$ dpkg -S $(which python3.6)
python3.6-minimal: /usr/bin/python3.6
$ python3.6 -m pytest --version
This is pytest version 3.3.2, imported from /usr/lib/python3/dist-packages/pytest.py

In that case, it'd be requisite that testing be done inside a venv per the CONTRIBUTING instructions.

We OK making that constraint? (and do we do that in CI?)

@EricCousineau-TRI
Copy link
Collaborator Author

EricCousineau-TRI commented Mar 29, 2021

Reviewing CI, seems like yeah, venv is always used. Kewl, will now constrain.

EDIT: Er, easy to do in Python code, less trivial in tracing back through CMake logic. Will need to revisit tomorrow or next week :(

@henryiii
Copy link
Collaborator

henryiii commented Apr 2, 2021

I like this solution; unlike a normal Python package, we are not able to require Python things; so being able to use pytest in the default environment for reasonably recent OS's (Ubuntu 18.04) is nice.

@henryiii
Copy link
Collaborator

henryiii commented Apr 2, 2021

I think we used to use the default pytest, and this got changed in the CI to force a newer pytest. As long as it's clearly marked to be removed when we do bump the minimum pytest, I think it's fine. This is useful for deploying pybind11 in package managers, where they generally use the system packages for dependencies.

Copy link
Collaborator

@rwgk rwgk left a comment

Choose a reason for hiding this comment

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

In support of Henry's approval.

@EricCousineau-TRI
Copy link
Collaborator Author

Gotcha, thank ya for the rationale and and history - and happy to merge in with less work haha.

@EricCousineau-TRI EricCousineau-TRI merged commit f676782 into pybind:master Apr 2, 2021
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label Apr 2, 2021
@henryiii henryiii removed the needs changelog Possibly needs a changelog entry label Jul 12, 2021
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.

[BUG] test_builtin_caster.test_int_convert: Fails on certain minor version of CPython 3.8?
3 participants