added smith waterman algorithm #9001
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
algorithms-keeper
bot
added
require descriptive names
![algorithms-keeper[bot]](https://avatars.githubusercontent.com/in/235834?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 reviewto trigger the checks for only added pull request files@algorithms-keeper review-allto 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.
CaedenPH
reviewed
Aug 22, 2023
tianyizheng02
requested changes
Sep 24, 2023
There was a problem hiding this comment.
Overall, I think the implementation looks good, but I have some general feedback:
- Can we make the score constants customizable? For example, we could have function parameters with default values so that the user can choose their score constants.
- Can we have more variety in the doctests? The current doctests are mostly variations on the same example input. The Wikipedia page for the Smith–Waterman algorithm has a more complex example, and that'd be good to add as a doctest (though the example uses different score constants).
algorithms-keeper
bot
removed
require descriptive names
tianyizheng02
requested changes
Sep 27, 2023
| return match if source_char == target_char else mismatch | ||
|
|
||
|
|
||
| def smith_waterman(query: str, subject: str) -> list[list[int]]: |
There was a problem hiding this comment.
Could you add optional parameters for the score constants in this function's header as well? Users will call this function specifically to run the algorithm, and currently there's no way for the user to pass in their desired score constants into this function.
There was a problem hiding this comment.
i hate repeating code, but there seems to be no way around it when it comes to default params
def smith_waterman(
query: str,
subject: str,
match: int = 1,
mismatch: int = -1,
gap: int = -2,
) -> list[list[int]]:
def score_function(
source_char: str,
target_char: str,
match: int = 1,
mismatch: int = -1,
gap: int = -2,
) -> int:
either way i got about it i'll have to unpack the kwargs in score function params or the body of the function and i think the params are a better way for clarity
tianyizheng02
approved these changes
Sep 30, 2023
algorithms-keeper
bot
removed
the
awaiting reviews
sedatguzelsemme
pushed a commit
to sedatguzelsemme/Python
that referenced
this pull request
Sep 15, 2024
* added smith waterman algorithm * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * descriptive names for the parameters a and b * doctesting lowercase upcase empty string cases * updated block quot,fixed traceback and doctests * shorter block quote Co-authored-by: Tianyi Zheng <[email protected]> * global vars to func params,more doctests * updated doctests * user access to SW params * formating --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Tianyi Zheng <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe your change:
Checklist: