Skip to content
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

fix: Fix clippy warnings #584

Open
wants to merge 44 commits into
base: master
Choose a base branch
from

Conversation

albertsgarde
Copy link

@albertsgarde albertsgarde commented Apr 7, 2024

Fixes all but one clippy warning throughout the repo.
Unfortunately, this means changing a lot of files, though luckily all changes are confined to only a few lines.

The only clippy warning left is the one complaining that the pairhmm module has a child called pairhmm. I didn't want to allow the warning, and changing the module name is not something I wanted to make a decision about.

I considered splitting this PR up on a per module basis in order to potentially make it easier to merge, but I thought I'd start off like this. Each commit only changes a single file and only fixes a single clippy warning, so it should be pretty simple to cherry pick.

Since `seq_pretty` and `sequences` have the same length, this should be
equivalent.
Also move the `is_empty` into the parentheses to simplify expression.
The original clippy lint was to use a `for` loop instead of a `while let`,
but I this was an obvious improvement as well.
This also allows destructuring the elements of `max_in_column`
into `score` and `col_max_row`, which makes the loop easier to read.
Also couldn't stop myself from renaming `j` to `col_index`.
Clippy has a warning against `vec!` macro calls with a single range as
argument. This is usually an error as described on their website, but in
this case it is in fact what we want, so we disable the warning for this
specific test.
Also use `zip` iterator instead of indexing.
@albertsgarde albertsgarde changed the title Fix clippy warnings fix: Fix clippy warnings Apr 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant