-
Notifications
You must be signed in to change notification settings - Fork 7.1k
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 broken python method #8821
Fix broken python method #8821
Conversation
@RDIL Is this Python version related? |
@iSazonov what do you mean? I’m just fixing broken code in this PR. |
I guess that the code worked on old Python version. |
The code clearly defines that the script should use a python 3.x.x version, but the this keyword hasn’t been a thing since the 2.5 series. I think it is time to update! |
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.
LGTM
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.
Worth noting that neither this
nor self
are keywords in Python; instance methods in Python pass the instance reference explicitly through the first parameter, and you can call that parameter whatever you like (you don't even need to be consistent):
class MyObject:
def __init__(banana):
banana.field = 10
def get_field(truck):
return truck.field
if __name__ == '__main__':
print(MyObject().get_field()) # prints '10'
So this Python script would have run correctly using this
.
However, self
is the near-universal convention, and worth us aligning to that, especially in our examples. Thanks @RDIL!
@SteveL-MSFT Make sense to move this to Docs repo? |
@iSazonov We may want to eventually have a doc page about interop with other languages, but we can keep this code here in this repo for now |
PR Summary
this
doesn’t work in python, the keyword isself
.PR Context
It needs to be fixed.
PR Checklist
.h
,.cpp
,.cs
,.ps1
and.psm1
files have the correct copyright headerWIP:
or[ WIP ]
to the beginning of the title (theWIP
bot will keep its status check atPending
while the prefix is present) and remove the prefix when the PR is ready.[feature]
to your commit messages if the change is significant or affects feature tests