Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add typing to
scope()
#1170Add typing to
scope()
#1170Changes from 1 commit
ff7b948
1c553e8
e1f3fd9
6e265d5
2d3f8ed
be92085
2260c12
84ddf63
e3c1471
c5bd321
b9e12ab
946352b
abb4033
38ad22c
cdc150b
af720ae
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Regarding
LocalsDictNodeNG
: I fear that at some point it will just create another circular dependency. Maybe we should start looking intoProtocol
classes for that 🤔 Haven't really done so myself, thus don't know for certain if it would work.https://docs.python.org/3/library/typing.html#typing.Protocol
--
Another issue: If at some point some scoped node inherits from another mixin class (not
LocalsDictNodeNG
, this will break.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.
Are you sure? Doesn't using
"Class"
prevent circular dependency?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.
The current code is fine. As you said, using a string as type annotation does prevent the circular dependency.
It's probably just that I'm not really a fan of the class name itself (
LocalsDictNodeNG
). Maybe that should be changed at some point 🤔Anyway, let's leave it for now.