-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Pull request regarding benchmarks and tests #102
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
Conversation
Checking the linter, a lot of the issues seem to stem from general assignment issues. I'll try to tackle those |
Everything else is a part of topological |
Gonna sleep on it. The main problem was this sort was written to accommodate multiple input possibilities and has multiple supporting functions to facilitate those options. Furthermore, because it will require its own special test suite utilizing a graph rather than a list, the whole topological_sort.py is more or less causing problems for the linter (see the note on how A bandaid fix is to effectively comment out the whole file while I build a suite for it then deal with the remainder of the tests and put TopSort off for a different PR. Up to you on how you want to proceed. |
Wrapped the whole thing in a giant comment to get things passing. Happy to tackle this however is deemed most professional, timely, and applicable, but figured something passing seemed to be the highest priority. |
As a follow-up to PR #100 . There are still some issues I am working out presently.
The main one is topological sort might need to be reworked in how it receives tests or it may need to have its own tests written up specifically for it, different from the normal format. This is due to topological taking in a slice of edges rather than a slice of ints.
Also, while not a huge issue, Selection sort seems unable to handle a sufficiently large sort so I may need to dig in and rewire the large tests to account for selection sort being slow.
I'll be passing some updates in the near future, but just wanted to get this on the books ASAP.