-
-
Notifications
You must be signed in to change notification settings - Fork 46.5k
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
Create count negative numbers in matrix algorithm #8813
Create count negative numbers in matrix algorithm #8813
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Click here to look at the relevant links ⬇️
🔗 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
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.
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Click here to look at the relevant links ⬇️
🔗 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
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.
Note this pr supposedly has lack of tests however the benchmark function shouldn't have doctests anyway |
When would we want to use this algorithm? |
@@ -0,0 +1,164 @@ | |||
""" | |||
Given an `m x n` matrix grid which is sorted in non-increasing order |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should there be a validate_matrix(matrix)
function? We do not need to run it everywhere but we should have it if we want to validate test input.
Co-authored-by: Christian Clauss <cclauss@me.com>
Co-authored-by: Christian Clauss <cclauss@me.com>
@cclauss If you have a grid of sorted numbers in descending order in a matrix, this algorithm can be used to count the number of negative numbers. Not sure when someone would want to use it, but then again when would someone want to use the quine from other/quine? To answer your previous comment about validating the matrix, I don't think that is necessary. For searches that require sorted arrays, there aren't checks to ensure that those arrays are sorted so I think indicating that the matrix must be sorted is good enough |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking cool!!
Co-authored-by: Christian Clauss <cclauss@me.com>
Co-authored-by: Christian Clauss <cclauss@me.com>
Co-authored-by: Christian Clauss <cclauss@me.com>
Co-authored-by: Christian Clauss <cclauss@me.com>
0d8a616
to
a55db73
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Click here to look at the relevant links ⬇️
🔗 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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Click here to look at the relevant links ⬇️
🔗 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
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.
@cclauss All changes have been implemented and tests are passing |
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Thanks for doing this.
I have never figured out https://docs.python.org/3/library/bisect.html but it would be interesting to benchmark against an implementation that leveraged it. |
* updating DIRECTORY.md * feat: Count negative numbers in sorted matrix * updating DIRECTORY.md * chore: Fix pre-commit * refactor: Combine functions into iteration * style: Reformat reference * feat: Add timings of each implementation * chore: Fix problems with algorithms-keeper bot * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * test: Remove doctest from benchmark function * Update matrix/count_negative_numbers_in_sorted_matrix.py Co-authored-by: Christian Clauss <cclauss@me.com> * Update matrix/count_negative_numbers_in_sorted_matrix.py Co-authored-by: Christian Clauss <cclauss@me.com> * Update matrix/count_negative_numbers_in_sorted_matrix.py Co-authored-by: Christian Clauss <cclauss@me.com> * Update matrix/count_negative_numbers_in_sorted_matrix.py Co-authored-by: Christian Clauss <cclauss@me.com> * Update matrix/count_negative_numbers_in_sorted_matrix.py Co-authored-by: Christian Clauss <cclauss@me.com> * Update matrix/count_negative_numbers_in_sorted_matrix.py Co-authored-by: Christian Clauss <cclauss@me.com> * refactor: Use sum instead of large iteration * refactor: Use len not sum * Update count_negative_numbers_in_sorted_matrix.py --------- Co-authored-by: github-actions <${GITHUB_ACTOR}@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Christian Clauss <cclauss@me.com>
Describe your change:
Three implementations of this algorithm are implemented and benchmarked together with doctests
Checklist:
Fixes: #{$ISSUE_NO}
.