[MRG] DOC: Add pytest version in documentation#12002
[MRG] DOC: Add pytest version in documentation#12002rth merged 7 commits intoscikit-learn:masterfrom
Conversation
README.rst
Outdated
| After installation, you can launch the test suite from outside the | ||
| source directory (you will need to have the ``pytest`` package installed):: | ||
| source directory (you will need to have the ``pytest`` package installed | ||
| (version >= 3.3.0)):: |
There was a problem hiding this comment.
maybe "you will need to have pytest >= 3.3 installed"
| <https://docs.pytest.org>`_ library. Some tests also require having | ||
| `pandas <https://pandas.pydata.org/>` installed. | ||
| <https://docs.pytest.org>`_ library (version >= 3.3.0). Some tests also require | ||
| having `pandas <https://pandas.pydata.org/>` installed. |
There was a problem hiding this comment.
Maybe
"Testing requires having pytest <https://docs.pytest.org>_ >= 3.3.0."
|
Can we assert this programmatically in |
something like this ? --- a/conftest.py
+++ b/conftest.py
@@ -12,6 +12,10 @@ import pytest
from _pytest.doctest import DoctestItem
+if LooseVersion(pytest.__version__) < '3.3.0':
+ raise('Your version of pytest is too old, you should have at least '
+ 'pytest >= 3.3.0 installed.')
+
def pytest_collection_modifyitems(config, items):
# FeatureHasher is not compatible with PyPy |
|
If the alternative is false negatives, that looks worthwhile
|
|
In any case as the CI version is up to date, I think false negatives would be detected before merging, but I guess this would indeed help contributors to avoid them before committing, when running test locally |
conftest.py
Outdated
| from _pytest.doctest import DoctestItem | ||
|
|
||
|
|
||
| if LooseVersion(pytest.__version__) < '3.3.0': |
There was a problem hiding this comment.
Make the min version a variable for a little future proofing
I tried to do this so as to only have to change it in If this is too much I can revert and only change If it is good I can also maybe try to do it for other version numbers in the project in another PR ? |
doc/conf.py
Outdated
| # loading variables | ||
| conftest_vars = {} | ||
| with open(os.path.join('..', 'conftest.py')) as fp: | ||
| exec(fp.read(), conftest_vars) |
There was a problem hiding this comment.
Not so keen on the exec, and avoiding the exec would probably lead to some possibly awkward imports / PATH issues.
Having this mechanism to avoid putting the version once in a rst is a bit too much IMO :) If the version changes we would have to change the readme anyway, so we might as well do it twice (which would need to be done twice in any case) . git grep 3.3.0 doesn't cost much..
There was a problem hiding this comment.
Better off having a sklearn/_requirements.py file than this... but this is all too much sophistication for the present PR. Thanks for trying to makde it maintainable!
|
I agree it was too much, I simplified the PR |
3 participants
This PR adds the minimal pytest version needed in order to avoid arguments like
match(as inwith pytest.raises(SomeError, match='some error message'):) to fail silently (see #12001)