-
Notifications
You must be signed in to change notification settings - Fork 12.1k
Fixed Python Code #1789
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
Fixed Python Code #1789
Conversation
Does their server even support a version of python that supports f-strings? AWS Lambda only supports up to 3.9 of python, which does not have f-strings, for instance. |
That's a brilliant question! I am sure the Python version they are using supports f-strings because some part of their code was already using f-strings (basically it was a mix of both f-strings and Also f-string have been there since Python 3.6 |
I also would suggest splitting up the PRs |
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.
Looks fine
This is quite a big PR and i agree with him that you should probably split your PR in order for the Twitter devs to better implement your changes. |
The way to split a PR like this up is in terms of structure:
As PRs are approved and merged, you rebase the remaining PRs |
Fixes: #1764 and #365
black
andisort
).format()
method with f-strings<<
instead of**
operator. For example2**n
can rewritten as1<<n
(resulting in operation time reducing by >40%)OrderedDict
with defaultdict