Drop python < 3.8 #117
Drop python < 3.8 #117
Conversation
| # FIXME: This is the only remaining usage for six. Do we still need this check? | ||
| # If so, then we should copy the function here and remove dependency on six | ||
| source_text = six.ensure_text(source_text) |
There was a problem hiding this comment.
Yes, I'd say we should still decode bytes to preserve compatibility. So the type hint needs to be amended. Also the "Decode source after parsing to let Python 2..." doesn't even make sense and needs updating.
| # TODO: since we don't need python 2 anymore, should we move testdata/python3/astroid to testdata/astroid? | ||
| return os.path.join(os.path.dirname(__file__), "testdata", "python3", *path_parts) |
There was a problem hiding this comment.
it wouldn't hurt, but it's not required
|
@alexmojaki @dsagal I don't have permissions to add you as reviewers but please look at this pr. |
| @@ -34,9 +28,7 @@ classifiers = | |||
|
|
|||
| [options] | |||
There was a problem hiding this comment.
| @@ -146,6 +145,7 @@ def test_unicode_offsets(self): | |||
|
|
|||
| def test_coding_declaration(self): | |||
| """ASTTokens should be able to parse a string with a coding declaration.""" | |||
| # TODO: figure out if the comment below is relevant | |||
There was a problem hiding this comment.
The Python 2 part can be removed, but let's test that it works with both bytes and str.
| # TODO: is this test still needed? | ||
| # This testcase imports print as function (using from __future__). Check that we can parse it. | ||
| # verify_all_nodes doesn't work on Python 2 because the print() call parsed in isolation | ||
| # is viewed as a Print node since it doesn't see the future import |
There was a problem hiding this comment.
| # TODO: is this test still needed? | |
| # This testcase imports print as function (using from __future__). Check that we can parse it. | |
| # verify_all_nodes doesn't work on Python 2 because the print() call parsed in isolation | |
| # is viewed as a Print node since it doesn't see the future import | |
| # This testcase imports print as function (using from __future__). Check that we can parse it. |
There was a problem hiding this comment.
yeah but why do we need to import print from future?
There was a problem hiding this comment.
This probably isn't testing anything interesting any more but we might as well keep it.
|
Thanks for your hard work, sorry it took so long to respond |
|
pîng |
|
Please add patch with filtering |
|
Sorry everyone this hasn't moved. @aqeelat , any chance you could merge or rebase onto master? A couple other PRs landed since you opened this, and I am getting merge conflicts. |
| install_requires = | ||
| six >= 1.12.0 | ||
| typing; python_version < "3.5" | ||
| install_requires = six >= 1.12.0 |
There was a problem hiding this comment.
Doesn't this PR actually make six irrelevant and unused ?
| install_requires = six >= 1.12.0 |
There was a problem hiding this comment.
I see. I expect applying #117 (comment) would solve that, but it could also be done as a follow up patch.
There was a problem hiding this comment.
Please have look on de0a2e7 where is patch which removes use of six from all places.
There was a problem hiding this comment.
I see "import six" immediately after following this link.
Closes #111
Todo: