- Notifications
You must be signed in to change notification settings - Fork673
fix(types): prevent __dir__ in RestObject from producing duplicates#1383
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Changes fromall commits
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -132,7 +132,7 @@ def __ne__(self, other: object) -> bool: | ||||||
returnsuper(RESTObject,self)!=other | ||||||
def__dir__(self): | ||||||
returnsuper(RESTObject,self).__dir__()|self.attributes.keys() | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Can the result be converted to a list? As that is what it returned before.
ContributorAuthor
|
returnsuper(RESTObject,self).__dir__()|self.attributes.keys() | |
returnlist(super(RESTObject,self).__dir__()|self.attributes.keys()) |
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 (for selfish reasons) would like it to return a list. As I am going to be working on adding type-hints to that code in the future 😊 But not necessary.
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.
And not necessary, because I think I will be adding a type-hint for the return value ofIterable[something]
, most likely.
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.
Sounds good to me. Feel free to apply the above suggestion (or not)
Looking forward to type hinting!
Worth mentioning that it'susually not necessary to provide type annotations or type hints for certain magic methods, since their return type is implied because there's no way to implement them to return a different type.
For example, the return type for methods like__str__
,__repr__
,__len__
,__dir__
, are known to IDEs and type checkers like MyPy automatically because the returned type must be valid, otherwise aTypeError
is raised.