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
Making typed.List a typing Generic #5958
Making typed.List a typing Generic #5958
Changes from 10 commits
3628b53
0857207
c0015eb
ece5174
2f96450
4d0db93
aebe1e9
fc3d5a0
c66c6b2
a7b88f9
a6fcb13
6529714
87fb3c7
4199351
2fd3e59
c4f0139
bb375b9
4c725e3
02fe12a
9838739
e8294ca
a8c8edc
f4635de
f511819
d9c80c2
1dfa46f
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.
Under strict mode inferred optionals are disabled, so these should be
Optional[int]
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.
thanks, I didn't realize that strict optionals was the default, I thought that inferred optionals were the default.
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 wonder if we shouldn't consider switching to
--no-strict-optional
to avoid the added verbosity. For example,index
could fit in one line (easier to read) without losing much in terms of typing (I think:int = None
is clear enough). what do you think?vs
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.
For internal annotations that's something we can decide on. I think for external facing annotations we should be as precise as possible to avoid incompatibilities for users who don't use
--no-strict-optional
.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.
you're right, I changed it to the explicit
Optional
.