Professor lupin#1038
Open
trishorts wants to merge 17 commits into
Open
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #1038 +/- ##
==========================================
+ Coverage 81.63% 81.74% +0.10%
==========================================
Files 369 371 +2
Lines 47605 48007 +402
Branches 5649 5718 +69
==========================================
+ Hits 38863 39243 +380
- Misses 7643 7658 +15
- Partials 1099 1106 +7
🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2599536d48
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This comment has been minimized.
This comment has been minimized.
Resolve the mzLib.nuspec conflict by taking master's bumped OpenMcdf (3.1.3) and OpenMcdf.Extensions (2.3.1) versions, matching the merged Readers.csproj package references. All other files auto-merged cleanly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…oducts - Proxy digestion: replicate OneBasedPossibleLocalizedModifications onto the second copy of the doubled sequence (key k in 1..N-1 also at k+N) so wrap-around linear fragments carry their annotated modifications and correct masses instead of silently dropping them on the wrapped portion. - Full-ring enumeration: merge OneBasedPossibleLocalizedModifications alongside the supplied variable mods (deduped by position+id), matching the linear digestion path so intact-ring forms keep annotated PTM isoforms. - Full-ring enumeration: cap modified forms at DigestionParams.MaxModificationIsoforms, mirroring the linear pathway and avoiding a combinatorial isoform explosion. - Full-ring enumeration: recompute NumFixedMods from the merged dictionary so a variable mod overriding a fixed mod at the same residue no longer overcounts. Addresses the Critical + two Major findings from the PR smith-chem-wisc#1038 re-review and the two Codex P1 comments on localized-PTM handling. All 98 circular tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@nbollis — heads up, this looks like it's the review for #1036 (the |
Four tests, each failing on the pre-fix code: - wrap-around fragment carries an annotated localized mod on its wrapped (second-copy) residue; - intact-ring form includes an annotated localized mod not passed as a variable mod; - full-ring variable-mod forms are bounded by MaxModificationIsoforms; - NumFixedMods is not overcounted when a variable mod overrides a fixed mod. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add four NUnit tests to CircularPeptideWithSetModifications: - constructor rejects N-terminal and C-terminal modifications (no free termini on a circular peptide); - FragmentInternally returns identical products on a repeat call (cached ring-mass / doubled-prefix arrays); - FragmentInternally enforces minLengthOfFragments >= 1. Lifts CircularPeptideWithSetModifications.cs patch coverage 82.9% -> 92.4%; all four files touched by the PR are now >= 90%. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This pull request introduces support for circular (head-to-tail cyclized) peptides in the proteomics library by adding a new
CircularPeptideWithSetModificationsclass and making key base class methods virtual to allow for specialized behavior. The main focus is enabling correct mass calculations and fragmentation logic for circular peptides, which differ from linear peptides due to their unique topology.Key changes include:
Support for Circular Peptides:
CircularPeptideWithSetModificationsinProteomics.ProteolyticDigestion, representing circular peptides with correct mass and fragmentation logic, including enforcement of no terminal modifications and specialized fragment enumeration.Extensibility for Specialized Peptide Types:
Digestmethod inProteinvirtual, allowing derived classes such asCircularProteinto override digestion behavior for specialized peptide types.MonoisotopicMassproperty inPeptideWithSetModificationsvirtual, enabling derived types to adjust mass calculations (e.g., subtracting water for circular peptides).FragmentInternallymethod inPeptideWithSetModificationsvirtual, so derived classes can implement topology-specific fragmentation logic.