#8 Declined
Repository
yasirs yasirs
Branch
default
Repository
pypy pypy
Branch
default

added numpy.sort, tests, and docstring for numpy.array.sort

Author
  1. yasirs avataryasirs
Reviewers
Description
No description

Comments (8)

    1. yasirs author

      Sure, do you want to let me know the concern so I can try to resolve it? (whether it is about conforming to cpython's numpy, or testing, performance, or architecture etc.)

      Thanks!

      ps. I sent this earlier from my inbox without realising that that was a private communication. Apologies if a double copy of this message arrives.

      1. Alex Gaynor

        Sorry, here are the issues I saw:

        • It would need to be rewritten after the merge of the dtypes branch (which rewrites most of everything :P)
        • It doesn't work with any of the virtual arrays (I'm surprised it even translates)
        • It looks like a pretty simple quicksort, which means it can have performance issues (I'd like to try using the TimSort we already have)
        • There are stylistic issues, semicolons, and excessive parens mostly.
        1. yasirs author

          I think the major issue is that of dtypes. I had imagined a number of passes to add other sorting algorithms and improvements through writing tests. I agree that it doesn't work for virtual arrays. (I think you mean stuff like slices, right?). I guess my brief question is, if people are interested in contributing to numpy/scipy development, are there specific helpful things to do? Perhaps a to do list with tests or a roadmap? Or just wait for the dtypes branch to be merged to the default and then go from there?

    2. Armin Rigo

      Oups, sorry about merging too eagerly the previous pull request.

      I can foresee a time when these pull requests will become annoying... yasirs should just be given access rights to the main repo, and just work in a branch.

    1. Pypy repo owner

      I don't think this is complete. The algorithm looks a bit to simple and also it can just reuse timsort from rlib with a little work. Also, as for a sorting algo reimplementation it definitely lacks tests (there is no odd-sized list, no empty list for one)

      Cheers, fijal

    2. yasirs author

      I abandoned it assuming these features should be added after merging the dtypes branch after reading Alex's comments. I'd still like to work on numpy development though.

Tip: Filter by directory path e.g. /media app.js to search for public/media/app.js.
Tip: Use camelCasing e.g. ProjME to search for ProjectModifiedEvent.java.
Tip: Filter by extension type e.g. /repo .js to search for all .js files in the /repo directory.
Tip: Separate your search with spaces e.g. /ssh pom.xml to search for src/ssh/pom.xml.
Tip: Use ↑ and ↓ arrow keys to navigate and return to view the file.
Tip: You can also navigate files with Ctrl+j (next) and Ctrl+k (previous) and view the file with Ctrl+o.
Tip: You can also navigate files with Alt+j (next) and Alt+k (previous) and view the file with Alt+o.