-
Notifications
You must be signed in to change notification settings - Fork 34
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
Put those mods back where they came from or so help me #827
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
Updating Master
Me - 0
This reverts commit f06cbc0.
Improved logic in `DigestionProduct.cs` to ensure correct application of N-terminal and C-terminal modifications to biopolymers, preventing overwriting unless the new modification is more specific. Updated assertions and modification order in test cases to reflect changes, enhancing accuracy and robustness of the digestion process.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #827 +/- ##
==========================================
+ Coverage 77.75% 77.76% +0.01%
==========================================
Files 230 230
Lines 34144 34166 +22
Branches 3534 3540 +6
==========================================
+ Hits 26547 26569 +22
Misses 6991 6991
Partials 606 606
|
Does this only apply to the behaviour of fixed mods? If you have peptide-N-term and protein-N-term variable mods, does one take precedence? |
trishorts
requested review from
Alexander-Sol,
pcruzparri,
zhuoxinshi and
RayMSMS
January 22, 2025 16:28
trishorts
approved these changes
Jan 22, 2025
Alexander-Sol
approved these changes
Jan 22, 2025
nbollis
changed the title
Put those mods back on the termini
Put those mods back where they came from or so help me
Jan 22, 2025
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.
My last PR made an incorrect assumption: modification localization restriction "peptide N-terminal" is meant to refer to ANY N-term, not just those that are generated during digestion.
In the last digestion PR, I made peptide N-terminal mod and N-terminal modification localization mutually exclusive.
This incorrect change results in no isobaric quantification labels being added to the peptide which contains the protein terminus.
The only integration test failing is due to the calibration change. This PR is safe
Changes
DigestionProduct.cs
: Ensured correct application and management of protease-associated and terminal modifications. By maintining current overwriting behavior, but ensuring that protein n-term overwrites peptide and not the other way around when applicable.ModificationLocalization.cs
: "Peptide N-terminal." mods are now valid on ALL N-termini.TestProteinDigestion.cs
: Updated and added test cases to reflect new terminal modification logic.TestDigestion.cs
: Adjusted tests for expected digestion products and sequences with terminal modifications.