Optimized recursive_bubble_sort #2410
Conversation
|
@cclauss Please review. Thanks in advance :) |
|
Hey @shellhub, TravisCI finished with status TravisBuddy Request Identifier: cb2f0a90-f1af-11ea-ba41-b1d3a642fad8 |
|
Hey @shellhub, TravisCI finished with status TravisBuddy Request Identifier: ec7590c0-f1af-11ea-ba41-b1d3a642fad8 |
|
Hey @shellhub, TravisCI finished with status TravisBuddy Request Identifier: 18cf7a50-f1b0-11ea-ba41-b1d3a642fad8 |
|
Hey @shellhub, TravisCI finished with status TravisBuddy Request Identifier: 3c4e1630-f1b0-11ea-ba41-b1d3a642fad8 |
Travis tests have failedHey @shellhub, TravisBuddy Request Identifier: 97257ab0-f1b2-11ea-ba41-b1d3a642fad8 |
|
@cclauss why Travic CI occur error? |
Travis tests have failedHey @shellhub, TravisBuddy Request Identifier: d29c1380-f1b5-11ea-ba41-b1d3a642fad8 |
|
Probably trailing whitespace deleted in doctests. |
|
How to solve？😭 |
|
See which files are failing in Travis CI and then undo the changes to those files. |
|
Yes. I found why build error was occur. Because whitespace deleted in doctests. Should I update origin source code and commit again? have you any better suggestion? @cclauss |
Travis tests have failedHey @shellhub, TravisBuddy Request Identifier: 5134a1b0-f1e5-11ea-ba41-b1d3a642fad8 |
Travis tests have failedHey @shellhub, TravisBuddy Request Identifier: ece4a470-f1ea-11ea-ba41-b1d3a642fad8 |
Travis tests have failedHey @shellhub, TravisBuddy Request Identifier: 58945cd0-f1ee-11ea-ba41-b1d3a642fad8 |
|
The build log is very difficult to understand. |
|
This was more than just running black. In the cases of the failing doctests, whitespace has been removed from the end of lines which the doctest needs to see for an exact match. This is one of the reasons why black will not suppress trailing whitespace that appears inside of triple quoted strings. There are three files that fail the Travis doctests. If the changes to those three files are reverted then the tests will pass again. |
Travis tests have failedHey @shellhub, TravisBuddy Request Identifier: c60c3520-f26c-11ea-82b8-f1691ab9f797 |
|
My sense is that you add the trailing whitespace and then the autoblack GitHub Action undoes it so the tests fail. |
|
@cclauss All checks have passed. Can you review again. Thanks :) |
|
I am completely lost by the change to bubble sort! We now require the user to provide |
Not exactly. return list_data if not swapped else bubble_sort(list_data, length - 1) |
|
Get a maximum value to the end of list for each recursive call. Then recursion sort left elements ( |
|
Please look at it again. Passing a number around but not actually using it (outside of decrementing it and passing it back to yourself) is a waste of cycles. |
|
@cclauss you are right. I forget make changes that you mentioned before. I make changes like this. Please review again. Thanks :) length = length or len(list_data)
swapped = False
for i in range(length - 1):
if list_data[i] > list_data[i + 1]:
list_data[i], list_data[i + 1] = list_data[i + 1], list_data[i]
swapped = True
return list_data if not swapped else bubble_sort(list_data, length - 1) |
|
I changed |
|
OK... Here is what we are going to do. I will land this now. From now on, no PRs that touch so many files. Please create a new PR for bubble sort that has both the old algorithm and the "oprimized" version in a single file with a timeit (or similar) benchmark that measures the relative performance of each. |
* optimized recursive_bubble_sort
* Fixed doctest error due whitespace
* reduce loop times for optimization
* fixup! Format Python code with psf/black push
Co-authored-by: github-actions <${GITHUB_ACTOR}@users.noreply.github.com>
Describe your change:
Checklist:
Fixes: #{$ISSUE_NO}.