The Wayback Machine - https://web.archive.org/web/20200831055635/https://github.com/TheAlgorithms/Python/pull/2337
Skip to content
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

@Sonic0588
Copy link

Sonic0588 commented Aug 18, 2020

Describe your change:

Added static type checking to min_cost_string_conversion.py towards issue #2128.

  • Add an algorithm?
  • Fix a bug or typo in an existing algorithm?
  • Documentation change?

Checklist:

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized.
  • I know that pull requests will not be merged if they fail the automated tests.
  • This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
  • All new Python files are placed inside an existing directory.
  • All filenames are in all lowercase characters with no spaces or dashes.
  • All functions and variable names follow Python naming conventions.
  • All function parameters and return values are annotated with Python type hints.
  • All functions have doctests that pass the automated testing.
  • All new algorithms have a URL in its comments that points to Wikipedia or other similar explanation.
  • If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.
@@ -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

This comment has been minimized.

@cclauss

cclauss Aug 21, 2020

Member

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?

This comment has been minimized.

@Sonic0588

Sonic0588 Aug 21, 2020

Author

Yes, agree with you. I've corrected that in the new commit.

seq = []
return seq
Comment on lines 55 to 56

This comment has been minimized.

@cclauss

cclauss Aug 21, 2020

Member
Suggested change
seq = []
return seq
return []
Sonic0588 added 2 commits Aug 21, 2020
first_string: str,
second_string: str,
Comment on lines 15 to 16

This comment has been minimized.

@cclauss

cclauss Aug 21, 2020

Member
Suggested change
first_string: str,
second_string: str,
source_string: str,
destination_string: str,

This comment has been minimized.

@Sonic0588

Sonic0588 Aug 21, 2020

Author

Done it

Sonic0588 and others added 2 commits Aug 21, 2020
Co-authored-by: Christian Clauss <[email protected]>
@TravisBuddy
Copy link

TravisBuddy commented Aug 21, 2020

Hey @Sonic0588,
Something went wrong with the build.

TravisCI finished with status errored, which means the build failed because of something unrelated to the tests, such as a problem with a dependency or the build process itself.

View build log

TravisBuddy Request Identifier: 6650c3e0-e3bb-11ea-ae1e-a7d25f4a9615
@Sonic0588 Sonic0588 requested a review from cclauss Aug 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
You can’t perform that action at this time.