-
-
Notifications
You must be signed in to change notification settings - Fork 46.6k
added type hints to strings/min_cost_string_conversion.py #2337
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
added type hints to strings/min_cost_string_conversion.py #2337
Conversation
@@ -9,7 +11,9 @@ | |||
""" | |||
|
|||
|
|||
def compute_transform_tables(X, Y, cC, cR, cD, cI): | |||
def compute_transform_tables( | |||
X: str, Y: str, cC: int, cR: int, cD: int, cI: 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.
Maybe it is just me but I hate these variable names. Uppercase characters are reserved for constants in Python and single letter and double letter variable names are so old school. They look kludgy in modern programming. Why make the caller/reader guess all the time?
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, agree with you. I've corrected that in the new commit.
This reverts commit 971c156.
first_string: str, | ||
second_string: str, |
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.
first_string: str, | |
second_string: str, | |
source_string: str, | |
destination_string: str, |
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.
Done it
Co-authored-by: Christian Clauss <[email protected]>
Hey @Sonic0588, TravisCI finished with status TravisBuddy Request Identifier: 6650c3e0-e3bb-11ea-ae1e-a7d25f4a9615 |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
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.
Thank you for your pull request!🤩
It would be even better if you can add a few doctests for this file. Take a look at our Contribution Guidelines and open a new pull request for it, if you want to do so! Thanks. |
…s#2337) * done * add types for local variables * Revert "add types for local variables" This reverts commit 971c156. * rename variables * Update strings/min_cost_string_conversion.py Co-authored-by: Christian Clauss <[email protected]> * rename strings * use flake8 * Update strings/min_cost_string_conversion.py Co-authored-by: Christian Clauss <[email protected]>
…s#2337) * done * add types for local variables * Revert "add types for local variables" This reverts commit 971c156. * rename variables * Update strings/min_cost_string_conversion.py Co-authored-by: Christian Clauss <[email protected]> * rename strings * use flake8 * Update strings/min_cost_string_conversion.py Co-authored-by: Christian Clauss <[email protected]>
Describe your change:
Added static type checking to min_cost_string_conversion.py towards issue #2128.
Checklist:
Fixes: #{$ISSUE_NO}
.