-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Drop six support, drop Py35 support #4006
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice cleanup, will merge when the test are fixed on master.
This PR is supposed to help with some of the broken tests after the sister PR was merged to astroid. There are still some other tests relating to spurious "unused-import" with the few tests that still use six (and only as a base class), maybe that is the issue at hand? At any rate there were some merge conflicts so I've rebased on latest master. |
[testoptions] | ||
requires = six |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not doing as in unpacking_non_sequence.py
and remove six
import ?
[testoptions] | ||
requires = six |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Idem
[testoptions] | ||
requires = six |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Idem.
In fact those tests are those which are failing on the CI after pylint-dev/astroid#841 has been merged...
@dgilman i'm sorry but i do not understand what is the criteria to remove or keep six import in unit test files. There are still a few unit test that import six and others that do not have this import anymore... |
At this point there are still a handful of packages that are pulling in six so it is not easy to get a six-free testing environment. Maybe in a few months or a year the community will start its detachment from six and this can come back in.
They're kept in there because astroid has special handling for six.with_metaclass, as documented here. astroid and pylint tests which touched with_metaclass are kept using six because you need it to exercise that special handling. I can't comment on the correctness or not of #841's handling of with_metaclass, but it does seem suspicious that the PR does not acknowledge the existing support or modify it in any way. I am going to push a change here that drops the |
Also, to clarify a bit more about six vs not six: if you look at The other tests have |
Closed in favor of #4091 |
This PR is the pylint sibling to pylint-dev/astroid#864 . It updates the pylint test suite to run successfully in environments where six is not installed. The build is expected to fail until that PR is merged in.
Note that there are still a bunch of tests that reference the six module. These tests still pass because pylint didn't have to import six in order to do its linting.
Because the astroid PR bumps the minimum required Python to 3.6 I have also added commits to drop Python 3.5 support here. The logic from the other PR: pytest recently updated to require Python 3.6+. Although there are older versions that support older Pythons those versions have a dependency on six, making it impossible to assert in CI that six is not installed and that pylint/astroid runs correctly when six is not installed. If you can't run pytest on a platform you can't really support it. Because Python 3.5 is at a hard EOL and completely unmaintained it's about time to drop Python 3.5 support anyway.
The removed tests are ones that were always skipped because they could only run under Pythons before 3.6. If pylint is run under a Python 3.x interpreter against a Python 2.x file they result in syntax errors instead of the relevant warnings/errors. Because pylint itself has run under Python 3.x only for a few releases now I think it's safe to drop these unit tests as the situations they test against can no longer be reached.
I did some investigation into the
--py3k
option per PCManticore's comments in the other PR. The feature appears to not be implemented using pip (anymore?) so there was nothing to change. While doing this investigation I found a unit test that was accidentally deleted but is still relevant for--py3k
users and brought it back in.The CHANGELOG and such are referencing a pylint 2.6.1 release but it seems like a change in minimum required Python version justifies a bump to pylint 2.7.0.
Steps
doc/whatsnew/<current release.rst>
.Description
Type of Changes
Related Issue