Skip to content

Creating a python example of quicksort #205

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

Closed
wants to merge 6 commits into from
Closed

Creating a python example of quicksort #205

wants to merge 6 commits into from

Conversation

KerimovEmil
Copy link

Creating a python example of quicksort, also based on the previous closed PR: #138 I also removed two unused lines in another python file.

@Butt4cak3 Butt4cak3 added Implementation This provides an implementation for an algorithm. (Code and maybe md files are edited.) Chapter This provides a new chapter. (md files are edited) labels Jul 2, 2018
@leios
Copy link
Member

leios commented Jul 2, 2018

Firstly, I really appreciate the contribution! However, as I mentioned in #163, the Algorithm Archive is (first and foremost) a book, which means that the text is an incredibly important part of this project. At this point in time, I am the only one who has written the bulk text for this project, and though I don't mind additional authors, additional chapters will be somewhat rigorously reviewed to make sure they follow at least a somewhat consistent style and make complete sense.

Since you did not write a chapter yet, I assume that you are waiting for the chapter to be written, which is fine. We will probably hold on to this PR until the quicksort chapter is written. We have 3 options:

  1. You keep re-writing the text until we are satisfied
  2. I rewrite the text and send a PR to you
  3. I rewrite the text, put it on the AAA and then you resubmit your code then

That said, this would be a test case for other "authors" in this project, so we are not sure how the process would work, exactly.

@KerimovEmil
Copy link
Author

Hi @leios , Thanks for your comment! I was actually using #163 as a reference before creating this PR. In it I saw that the last comment proposed contributing the code and creating a stub .md file. So that when the chapter would be written, some sample python code would already exist.

With that said, I'm open to any suggestion on how you want to proceed. All three options are fine to me.

On a possibly unrelated note, is there a way to suggest chapters to be made?

@Butt4cak3
Copy link
Contributor

Butt4cak3 commented Jul 3, 2018

The one who suggested to leave a stub article and add the code was me. It was probably not very clear, but it was meant as an exception, because the PR already existed and we didn't want to just throw the code away. It was not meant to be the new process for every chapter in the future.

On a possibly unrelated note, is there a way to suggest chapters to be made?

Every few weeks there is a poll on which topic to cover next on stream. The options in the poll are curated by @leios.

Copy link
Contributor

@zsparal zsparal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My main problem with this implementation is that:

  1. It's very wasteful. There's no reason to create 3 lists per quick_sort call for the algorithm to work. In fact, it completely ruins the algorithm's space complexity
  2. Somewhat related, but it's not the standard algorithm: it doesn't partition the elements in-place. Yes, you can make some very elegant implementations if you disregard parts of the problem but I don't think it's something we should do.

@Butt4cak3
Copy link
Contributor

What's the status of this? Can I close this, @leios? The original author seems to be inactive and the code isn't good according to @Gustorn.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Chapter This provides a new chapter. (md files are edited) Implementation This provides an implementation for an algorithm. (Code and maybe md files are edited.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants