Fixed#2642: Convert py module references to six module#2650
Fixed#2642: Convert py module references to six module#2650RonnyPfannschmidt merged 2 commits intopytest-dev:featuresfrom srinivasreddy:2642
Conversation
srinivasreddy
changed the title
|
This should definitely be done on the |
RonnyPfannschmidt
left a comment
RonnyPfannschmidt
left a comment
There was a problem hiding this comment.
nicely done
this beings us one step closer to no longer needing pylib
_pytest/nose.py
Outdated
| method = getattr(obj, name, None) | ||
| isfixture = hasattr(method, "_pytestfixturefunction") | ||
| if method is not None and not isfixture and py.builtin.callable(method): | ||
| if method is not None and not isfixture and six.callable(method): |
There was a problem hiding this comment.
i believe the callable builtin is back on all supported python versions so should be able drop the six reference here
There was a problem hiding this comment.
Yep, from the six docs:
Note callable has returned in Python 3.2, so using six’s version is only necessary when supporting Python 3.0 or 3.1.
setup.py
Outdated
|
|
||
| def main(): | ||
| install_requires = ['py>=1.4.33', 'setuptools'] # pluggy is vendored in _pytest.vendored_packages | ||
| install_requires = ['py>=1.4.33', 'six','setuptools'] # pluggy is vendored in _pytest.vendored_packages |
There was a problem hiding this comment.
i do wonder if we should have a minimal version there,
i suspect its necessary once we sort out _pytest.compat
| obj = (obj,) | ||
| # explicit naming | ||
| if isinstance(obj[0], py.builtin._basestring): | ||
| if isinstance(obj[0], six.string_types): |
There was a problem hiding this comment.
i just noticed the missing import here, please add it
|
|
||
| def main(): | ||
| install_requires = ['py>=1.4.33', 'setuptools'] # pluggy is vendored in _pytest.vendored_packages | ||
| install_requires = ['py>=1.4.33', 'six>=1.10.0','setuptools'] # pluggy is vendored in _pytest.vendored_packages |
|
well done, after the last import linting issue is resolved, we can take a look at vendoring six in order to protect ourselves against certain scenarios |
srinivasreddy
changed the title
|
Thanks @srinivasreddy! The only thing missing is a changelog entry, I suggest |
|
Btw, we can merge this as is and decide later if we need to vendor |
|
I think i am done with this PR. Whatever changes needed to remove py lib completely, can be done in another PR. 👍 |
Definitely, we appreciate it! Just pushed the changelog myself, I think this can be merged. Thanks again @srinivasreddy! |
|
@srinivasreddy well done, good work and thank you for preserving 👍 |
5 participants
No description provided.