-
Notifications
You must be signed in to change notification settings - Fork 4
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
chore: apply flake8 builtin rules #248
Conversation
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
ceb3331
to
5acdd24
Compare
ddbae38
to
571177b
Compare
def create(self, input: io.BufferedReader | bytes | str) -> Bundle: | ||
def create(self, archive: io.BufferedReader | bytes | str) -> Bundle: |
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.
Is this to avoid using input
as an argument?
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.
Correct. Since input
is a builtin function.
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.
We might not need to worry cause this is internal enough or no one is yet using it, but does this change have user-facing implications?
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.
Yes, it does if anyone is using kwargs when calling this method. Since it's early and we are pre-1.0.0, I am ok releasing this under 0.4.0 along with documentation of the breaking changes.
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 for my own edification: documentation of those breaking changes isn't in this PR yeah? That'll be a follow on?
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.
Yes, I think release notes are the correct place to write them.
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 started a migration guide in the changelog: https://github.com/posit-dev/posit-sdk-py/releases
Adds formatting rule and fixes variable names that shadow Python builtins.