-
-
Notifications
You must be signed in to change notification settings - Fork 276
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
Fix typing alias #916
Fix typing alias #916
Conversation
@Pierre-Sassoulas It would be great if you could look at over one last time. Just to make sure I didn't miss something obvious. I'm running some tests at the moment to check if everything is good. |
68e3f51
to
d112850
Compare
This could benefit from a rewrite using pre-commit for each commit: |
Or maybe we just squash. |
I would do a squash merge
The version needs to be updated, but otherwise I believe that's it. -- I noticed a moment ago the the |
Strange, |
Unfortunately some more errors, this time with pylint: https://github.com/cdce8p/pylint/runs/1999076587?check_suite_focus=true#step:6:205 |
and "ABCMeta" == meta.basenames[0] | ||
and meta.locals.get("__getitem__") is not None | ||
) | ||
assert isinstance(meta, nodes.ClassDef) |
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.
👍
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.
I really hope all the tests pass now. Was a bit more difficult to fix than I expected.
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.
@Pierre-Sassoulas Can you reproduce the Travis issue. I unfortunately can't 😞
https://travis-ci.org/github/PyCQA/astroid/jobs/760830892
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.
Hmm it's strange, on 2391061 I have an error but not the same than for Travis
____ ExceptionModelTest.test_oserror __
self = <tests.unittest_object_model.ExceptionModelTest testMethod=test_oserror>
def test_oserror(self):
ast_nodes = builder.extract_node(
"""
try:
raise OSError("a")
except OSError as err:
err.filename #@
err.filename2 #@
err.errno #@
"""
)
expected_values = ["", "", 0]
for node, value in zip(ast_nodes, expected_values):
inferred = next(node.infer())
> assert isinstance(inferred, astroid.Const)
E AssertionError: assert False
E + where False = isinstance(Uninferable, <class 'astroid.node_classes.Const'>)
E + where <class 'astroid.node_classes.Const'> = astroid.Const
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.
Hmm. Should I just change the required version to 3.8
?
Since it works with AppVeyor
Link, I would assume that this is a Travis issue.
… infer the origin class : i.e MutableSet = _alias(collections.MutableSet ...) origin is the class in collections module. Needs to add __getitem method on its metaclass so that is support indexing (MutableSet[T]).
2391061
to
56c7d24
Compare
@Pierre-Sassoulas I think it's done. Nothing unexpected with any tests. |
Sounds good, I'll release astroid 2.5.1 asap :) |
Steps
Description
Replaces #913
CC: @hippo91 @Pierre-Sassoulas
Type of Changes
Related Issue
Closes #905
Closes pylint-dev/pylint#4093
Closes pylint-dev/pylint#4131
Closes pylint-dev/pylint#4145
Closes pylint-dev/pylint#3247
Closes pylint-dev/pylint#2717