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

Ensure library matching uses the mass with the lowest mass error in case of ties #22

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

alex-l-kong
Copy link
Contributor

What is the purpose of this PR?

The _matching_vec helper function for library_matching previously assumed that there would only be one instance of a matching mass, so it took the first one every time. That can no longer be assumed: we will need to find the mass with the lowest mass error relative to ppm since there could be multiple masses.

How did you implement your changes

Implement an additional control flow that iterates through each mass and finds the minimum across all masses in library_peak_df.

Remaining issues

This is inherently a slower implementation since matches will take as long as non-matches. To speed this up, potentially iterate through a pre-sorted library_peak_df by m/z value.

@alex-l-kong alex-l-kong self-assigned this Apr 15, 2024
@coveralls
Copy link

Pull Request Test Coverage Report for Build 8697236362

Details

  • 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.4%) to 96.41%

Totals Coverage Status
Change from base Build 6777233943: -0.4%
Covered Lines: 157
Relevant Lines: 158

💛 - Coveralls

@alex-l-kong alex-l-kong requested review from kxleow and removed request for kxleow September 19, 2024 22:26
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.

2 participants