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: nucleotide symbol equals with dot #341

Merged
merged 1 commit into from
Mar 5, 2024

Conversation

JonasKellerer
Copy link
Contributor

From GenSpectrum/cov-spectrum-website#938

Summary

'.' should count if the symbol is equal to the reference genome. This PR should fix that.

PR Checklist

- [ ] All necessary documentation has been adapted or there is an issue to do so.

  • The implemented feature is covered by an appropriate test.

@@ -7,7 +7,7 @@ TEST(NucleotideSymbol, enumShouldHaveSameLengthAsArrayOfSymbols) {
}

TEST(NucleotideSymbol, conversionFromCharacter) {
EXPECT_EQ(silo::Nucleotide::charToSymbol('.'), silo::Nucleotide::Symbol::GAP);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see, it worked as the tests suggested, just the test was wrong 😄

@JonasKellerer
Copy link
Contributor Author

I will now also add that the amino acid symbols are treated the same. (They give the correct result, but not explicitly by design.)

@corneliusroemer
Copy link
Contributor

@JonasKellerer great yes, amino acid syntax should be the same, . equal to reference.

@JonasKellerer
Copy link
Contributor Author

There is now a new ticket #342 to revisit charToSymbol so it does not return the same result for an invalid char and for '.'.

Copy link
Contributor

@fengelniederhammer fengelniederhammer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@JonasKellerer JonasKellerer merged commit 6ad623e into main Mar 5, 2024
6 checks passed
@JonasKellerer JonasKellerer deleted the fixNucleotideEqualsWithDot branch March 5, 2024 15:51
fengelniederhammer added a commit to GenSpectrum/LAPIS that referenced this pull request Mar 6, 2024
fengelniederhammer added a commit to GenSpectrum/LAPIS that referenced this pull request Mar 6, 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.

3 participants