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

Create intro_sort.py #3877

Merged
merged 9 commits into from Nov 25, 2020
Merged

Create intro_sort.py #3877

merged 9 commits into from Nov 25, 2020

Conversation

@YeonJeongLee00
Copy link
Contributor

@YeonJeongLee00 YeonJeongLee00 commented Nov 11, 2020

Describe your change:

added introsort algorithm implementation to sorts

  • 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}.
@YeonJeongLee00 YeonJeongLee00 requested a review from mateuszz0000 as a code owner Nov 11, 2020
Copy link
Member

@mateuszz0000 mateuszz0000 left a comment

Please add doctests and typehints

black intro_sort.py
@YeonJeongLee00
Copy link
Contributor Author

@YeonJeongLee00 YeonJeongLee00 commented Nov 13, 2020

@mateuszz0000 I added doctests and typehints. Can you check my code?

Copy link
Contributor

@NumberPiOso NumberPiOso left a comment

Could you please add tests for different examples.

  • Large numbers
  • Small numbers
  • Empty list
  • Single list
  • Floating-point numbers
  • Strings
@YeonJeongLee00
Copy link
Contributor Author

@YeonJeongLee00 YeonJeongLee00 commented Nov 15, 2020

@NumberPiOso added more test

@YeonJeongLee00
Copy link
Contributor Author

@YeonJeongLee00 YeonJeongLee00 commented Nov 18, 2020

@mateuszz0000 @shellhub Can you review my code?

@JiHyunSong
Copy link

@JiHyunSong JiHyunSong commented Nov 22, 2020

@cclauss excuse me sir, can you reivew this code?

Copy link
Member

@cclauss cclauss left a comment

All functions should have tests. Maybe create one or more global test_arrays and test all functions against those common datasets.

sorts/intro_sort.py Outdated Show resolved Hide resolved
sorts/intro_sort.py Show resolved Hide resolved
sorts/intro_sort.py Show resolved Hide resolved
sorts/intro_sort.py Show resolved Hide resolved
sorts/intro_sort.py Show resolved Hide resolved
sorts/intro_sort.py Outdated Show resolved Hide resolved
sorts/intro_sort.py Outdated Show resolved Hide resolved
added doctest, modified code
sorts/intro_sort.py Outdated Show resolved Hide resolved
sorts/intro_sort.py Outdated Show resolved Hide resolved
def insertion_sort(array: list, start: int, end: int) -> list:
"""
>>> array = [4, 2, 6, 8, 1, 7, 8, 22, 14, 56, 27, 79, 23, 45, 14, 12]
>>> insertion_sort(array, 0, len(array))
[1, 2, 4, 6, 7, 8, 8, 12, 14, 14, 22, 23, 27, 45, 56, 79]
"""
for i in range(start, end):
Comment on lines 9 to 16

This comment has been minimized.

@cclauss

cclauss Nov 24, 2020
Member

Let's also enable the caller to simply do insertion_sort(my_array) and get back the expected result.

Suggested change
def insertion_sort(array: list, start: int, end: int) -> list:
"""
>>> array = [4, 2, 6, 8, 1, 7, 8, 22, 14, 56, 27, 79, 23, 45, 14, 12]
>>> insertion_sort(array, 0, len(array))
[1, 2, 4, 6, 7, 8, 8, 12, 14, 14, 22, 23, 27, 45, 56, 79]
"""
for i in range(start, end):
def insertion_sort(array: list, start: int = 0, end: int = 0) -> list:
"""
>>> array = [4, 2, 6, 8, 1, 7, 8, 22, 14, 56, 27, 79, 23, 45, 14, 12]
>>> insertion_sort(array, 0, len(array))
[1, 2, 4, 6, 7, 8, 8, 12, 14, 14, 22, 23, 27, 45, 56, 79]
"""
end = end or len(array)
for i in range(start, end):

This comment has been minimized.

@cclauss

cclauss Nov 24, 2020
Member

What happens if:

  1. start is a negative number
  2. if end is larger than len(array)
  3. start > end?
Copy link
Member

@cclauss cclauss left a comment

This is looking really cool.. One last thing to try before we land it. Thanks for your hard work!

Copy link
Member

@cclauss cclauss left a comment

Thanks much for your hard work here... I am going to accept and merge this but I still have reservations about insertion_sort()...

What happens if:

  • start is a negative number
  • if end is larger than len(array)
  • start > end

If you wanted to address these issues in a future pull request then I would be happy to review.

@cclauss cclauss merged commit 2b50aaf into TheAlgorithms:master Nov 25, 2020
2 checks passed
2 checks passed
build
Details
pre-commit
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.