Add a recursive merge sort algorithm that accepts an array as input. #4462

benfein · 2021-05-28T23:35:26Z

Add a recursive merge sort algorithm that accepts an array as input.

Describe your change:

Add a recursive merge sort algorithm that accepts an array as input.

[x ] Add an algorithm?
Fix a bug or typo in an existing algorithm?
Documentation change?

Checklist:

[x ] 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.
[x ] All new Python files are placed inside an existing directory.
[x ] All filenames are in all lowercase characters with no spaces or dashes.
[x ] All functions and variable names follow Python naming conventions.
[ x] All function parameters and return values are annotated with Python type hints.
[ x] All functions have doctests that pass the automated testing.
[x ] All new algorithms have a URL in its comments that points to Wikipedia or other similar explanation.
[x ] If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

Recursive Merge Sort That Accepts an Array

ghost

Click here to look at the relevant links ⬇️

🔗 Relevant Links

Repository:

Contributing guidelines

Project Euler solution guidelines

Python:

Formatted string literals (f-strings)

Type hints

doctest

unittest

pytest

Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.

algorithms-keeper commands and options

algorithms-keeper actions can be triggered by commenting on this PR:

@algorithms-keeper review to trigger the checks for only added pull request files

@algorithms-keeper review-all to trigger the checks for all the pull request files, including the modified files. As we cannot post review comments on lines not part of the diff, this command will post all the messages in one comment.

NOTE: Commands are in beta and so this feature is restricted only to a member or owner of the organization.

sorts/recursive_mergesort_array.py

ghost · 2021-05-28T23:35:42Z

sorts/recursive_mergesort_array.py

+		middle = len(arr) // 2
+		left = arr[:middle]
+		right = arr[middle:]
+		leftSize = len(left)


Variable and function names should follow the snake_case naming convention. Please update the following name accordingly: leftSize

ghost · 2021-05-28T23:35:42Z

sorts/recursive_mergesort_array.py

+		left = arr[:middle]
+		right = arr[middle:]
+		leftSize = len(left)
+		rightSize = len(right)


Variable and function names should follow the snake_case naming convention. Please update the following name accordingly: rightSize

mrmaxguns

Hello! Thanks for your contribution. This is my review of the code.

The two main things are:

If statements are usually written as if EXPRESSION and not if(EXPRESSION)
Incrementing a variable and assigning the result to it can be done with +=

If you disagree with any review, please leave a comment. Thanks!

sorts/recursive_mergesort_array.py

Co-authored-by: Maxim R. <49735721+mrmaxguns@users.noreply.github.com>

mrmaxguns · 2021-05-29T16:28:02Z

Please run Black on your code. Pre-commit failed because Black found issues. Install black with pip install black and run it with black file_name.py.

mrmaxguns · 2021-05-29T16:42:06Z

Flake8 failed because:

 flake8...................................................................Failed
- hook id: flake8
- duration: 8.47s
- exit code: 1

sorts/recursive_mergesort_array.py:16:9: E741 ambiguous variable name 'l'
sorts/recursive_mergesort_array.py:16:9: F841 local variable 'l' is assigned to but never used
sorts/recursive_mergesort_array.py:17:9: F841 local variable 'r' is assigned to but never used

mrmaxguns

A final review. Please add a docstring to the function briefly describing the mergesort method (and a link to the Wikipedia article for it). Also, please add comments describing various parts of the function (this doesn't have to be excessive, but should help the reader better understand what is happening).

A mergesort function has already been implemented, but I believe this can be merged after review, as the method of implementing the algorithm is different. Thank you for your contribution! 👍

sorts/recursive_mergesort_array.py

ghost

Click here to look at the relevant links ⬇️

🔗 Relevant Links

Repository:

Contributing guidelines

Project Euler solution guidelines

Python:

Formatted string literals (f-strings)

Type hints

doctest

unittest

pytest

Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.

algorithms-keeper commands and options

algorithms-keeper actions can be triggered by commenting on this PR:

@algorithms-keeper review to trigger the checks for only added pull request files

@algorithms-keeper review-all to trigger the checks for all the pull request files, including the modified files. As we cannot post review comments on lines not part of the diff, this command will post all the messages in one comment.

NOTE: Commands are in beta and so this feature is restricted only to a member or owner of the organization.

sorts/recursive_mergesort_array.py

mrmaxguns · 2021-05-30T01:53:28Z

I apologize for the wait. Pre-commit failed because of the comments. It wants the comments in the format # and not ##.

sorts/recursive_mergesort_array.py

Co-authored-by: Maxim R. <49735721+mrmaxguns@users.noreply.github.com>

mrmaxguns · 2021-05-30T15:28:56Z

Thank you for your contribution 🎆

…heAlgorithms#4462) This is a different recursive implementation of the merge sort algorithm. * Recursive Merge Sort That Accepts an Array Recursive Merge Sort That Accepts an Array * Add Wikipedia Link * Fixes naming conventions * Update sorts/recursive_mergesort_array.py Co-authored-by: Maxim R. <49735721+mrmaxguns@users.noreply.github.com> * Update sorts/recursive_mergesort_array.py Co-authored-by: Maxim R. <49735721+mrmaxguns@users.noreply.github.com> * Update sorts/recursive_mergesort_array.py Co-authored-by: Maxim R. <49735721+mrmaxguns@users.noreply.github.com> * Update sorts/recursive_mergesort_array.py Co-authored-by: Maxim R. <49735721+mrmaxguns@users.noreply.github.com> * Update sorts/recursive_mergesort_array.py Co-authored-by: Maxim R. <49735721+mrmaxguns@users.noreply.github.com> * Adds black format * Removes unused variables * Fixes variable names and adds documentation * Fixes variable names to use snake_case. * Removes double #. * Update sorts/recursive_mergesort_array.py Co-authored-by: Maxim R. <49735721+mrmaxguns@users.noreply.github.com> Co-authored-by: Maxim R. <49735721+mrmaxguns@users.noreply.github.com> Co-authored-by: Benjamin Fein <benfein78@icloud.com>

benfein added 2 commits May 28, 2021 19:31

Recursive Merge Sort That Accepts an Array

ad4b522

Recursive Merge Sort That Accepts an Array

Add Wikipedia Link

efa34bd

benfein requested a review from l3str4nge as a code owner May 28, 2021 23:35

ghost added the require type hints https://docs.python.org/3/library/typing.html label May 28, 2021

ghost reviewed May 28, 2021

View reviewed changes

ghost added the awaiting reviews This PR is ready to be reviewed label May 28, 2021

Fixes naming conventions

d022cfa

ghost removed the require type hints https://docs.python.org/3/library/typing.html label May 28, 2021

mrmaxguns suggested changes May 29, 2021

View reviewed changes

ghost added awaiting changes A maintainer has requested changes to this PR and removed awaiting reviews This PR is ready to be reviewed labels May 29, 2021

Update sorts/recursive_mergesort_array.py

7023837

Co-authored-by: Maxim R. <49735721+mrmaxguns@users.noreply.github.com>

ghost added awaiting reviews This PR is ready to be reviewed and removed awaiting changes A maintainer has requested changes to this PR labels May 29, 2021

benfein and others added 4 commits May 29, 2021 11:49

Update sorts/recursive_mergesort_array.py

c20610e

Co-authored-by: Maxim R. <49735721+mrmaxguns@users.noreply.github.com>

Update sorts/recursive_mergesort_array.py

13c9042

Co-authored-by: Maxim R. <49735721+mrmaxguns@users.noreply.github.com>

Update sorts/recursive_mergesort_array.py

999e144

Co-authored-by: Maxim R. <49735721+mrmaxguns@users.noreply.github.com>

Update sorts/recursive_mergesort_array.py

97e9423

Co-authored-by: Maxim R. <49735721+mrmaxguns@users.noreply.github.com>

mrmaxguns approved these changes May 29, 2021

View reviewed changes

ghost removed the awaiting reviews This PR is ready to be reviewed label May 29, 2021

ghost added the tests are failing Do not merge until tests pass label May 29, 2021

Adds black format

d92f687

ghost added the awaiting reviews This PR is ready to be reviewed label May 29, 2021

Removes unused variables

94a9323

ghost removed the tests are failing Do not merge until tests pass label May 29, 2021

mrmaxguns suggested changes May 29, 2021

View reviewed changes

sorts/recursive_mergesort_array.py Show resolved Hide resolved

sorts/recursive_mergesort_array.py Outdated Show resolved Hide resolved

ghost added awaiting changes A maintainer has requested changes to this PR and removed awaiting reviews This PR is ready to be reviewed labels May 29, 2021

Fixes variable names and adds documentation

751e9b8

ghost reviewed May 29, 2021

View reviewed changes

ghost added awaiting reviews This PR is ready to be reviewed and removed awaiting changes A maintainer has requested changes to this PR labels May 29, 2021

Fixes variable names to use snake_case.

f087db3

ghost added the tests are failing Do not merge until tests pass label May 30, 2021

Removes double #.

5a0c7ec

mrmaxguns reviewed May 30, 2021

View reviewed changes

sorts/recursive_mergesort_array.py Outdated Show resolved Hide resolved

Update sorts/recursive_mergesort_array.py

6d978ab

Co-authored-by: Maxim R. <49735721+mrmaxguns@users.noreply.github.com>

ghost removed the tests are failing Do not merge until tests pass label May 30, 2021

mrmaxguns approved these changes May 30, 2021

View reviewed changes

ghost removed the awaiting reviews This PR is ready to be reviewed label May 30, 2021

mrmaxguns merged commit 650039a into TheAlgorithms:master May 30, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a recursive merge sort algorithm that accepts an array as input. #4462

Add a recursive merge sort algorithm that accepts an array as input. #4462

benfein commented May 28, 2021 •

edited

Loading

ghost left a comment

ghost May 28, 2021

ghost May 28, 2021

mrmaxguns left a comment

mrmaxguns commented May 29, 2021 •

edited

Loading

mrmaxguns commented May 29, 2021

mrmaxguns left a comment

ghost left a comment

mrmaxguns commented May 30, 2021 •

edited

Loading

mrmaxguns commented May 30, 2021

Add a recursive merge sort algorithm that accepts an array as input. #4462

Add a recursive merge sort algorithm that accepts an array as input. #4462

Conversation

benfein commented May 28, 2021 • edited Loading

Describe your change:

Checklist:

ghost left a comment

Choose a reason for hiding this comment

🔗 Relevant Links

Repository:

Python:

Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.

algorithms-keeper actions can be triggered by commenting on this PR:

ghost May 28, 2021

Choose a reason for hiding this comment

ghost May 28, 2021

Choose a reason for hiding this comment

mrmaxguns left a comment

Choose a reason for hiding this comment

mrmaxguns commented May 29, 2021 • edited Loading

mrmaxguns commented May 29, 2021

mrmaxguns left a comment

Choose a reason for hiding this comment

ghost left a comment

Choose a reason for hiding this comment

🔗 Relevant Links

Repository:

Python:

Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.

algorithms-keeper actions can be triggered by commenting on this PR:

mrmaxguns commented May 30, 2021 • edited Loading

mrmaxguns commented May 30, 2021

benfein commented May 28, 2021 •

edited

Loading

mrmaxguns commented May 29, 2021 •

edited

Loading

mrmaxguns commented May 30, 2021 •

edited

Loading