-
Notifications
You must be signed in to change notification settings - Fork 46
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
Kdn adding and diagnostic filtering #2383
Kdn adding and diagnostic filtering #2383
Conversation
…Psms, Level1-glycoPsms) into the "AllResult.txt" file for glycoSearch (1) adding text function in PostGlycoSearchAnalysisTask class (2) adding tester in TestOGlyco class (make sure we parse the certain value) (3) revise the "readCsv", enable to read the allPSMs file smoothly.
…aMorpheus into glyco-search-comment
We also allow to get the same PSMs in duplicated files.
(1) Rewrite the Summary writing function (2)Add the <summary> comment in the fuction header
…aMorpheus into glyco-search-comment
1. add new tester model for "OGlycanCompositionFragments"
1. add the tester for writing function, in different search type 2. glycoBox tester for decoy glycanBox
improve the converage
1. new function "DiagonsticFilter" was built and corresponding test "IsobaricCase" was built 2. "SpectralRecoveryTest" fixing, the set up function will automatically clean the folder when we find the extra file in that.
…com/RayMSMS/MetaMorpheus into Kdn-Adding-and-diagnostic-filtering
1. delete the bin and set the file "copy always" 2. revise the "MetaDrawSettingAndViewsTest", detailed informed on comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall. I had a few questions and concerns that I left as comments throughout the PR. I also checked the code coverage and did not find an area where it would be easy to test the additions that you made.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be a separate file or should it be added to one of the existing glyco files?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just added to the existing glyco files. The Glycan_Mods is a default glycan database collection for the user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be a separate file or should it be added to one of the existing .gdb files?
|
||
//TO DO: Decoy O-glycan can be created, but the results need to be reasoned. | ||
//public static int[] SugarShift = new int[]{ -16205282, -20307937, -29109542, -14605791, -30709033, -15005282, -36513219, -40615874, 16205282, 20307937, 29109542, 14605791, 30709033, 15005282, 36513219, 40615874 }; | ||
private readonly static int[] SugarShift = new int[] | ||
private readonly static int[] SugarShift = new int[] //still unclear about the shift... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are we still unclear about this shift? Can you add a comment about what these numbers are and how they are used?
/// Constructor of GlycanBox. | ||
/// </summary> | ||
/// <param name="ids"> The glycanBox composition, each number represent one glycan index in the database</param> | ||
/// <param name="targetDecoy"></param> | ||
public GlycanBox(int[] ids, bool targetDecoy = true):base(ids) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bool targetDecoy should be changed to isTarget to make it more clear if true is a target or true is a decoy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corrected it. Thanks
@@ -207,7 +240,7 @@ public static List<GlycanIon> NGlycanCompositionFragments(byte[] kind) | |||
|
|||
for (int add_fuc_count = 2; add_fuc_count <= fuc_count; add_fuc_count++) | |||
{ | |||
GlycanIon add_fuc_glycanIon = ExtendGlycanIon(hexose_glycanIon, 0, 0, (byte)add_fuc_count, 0, glycan_mass); | |||
GlycanIon add_fuc_glycanIon = ExtendGlycanIon(hexose_glycanIon, 0, 0, 1, 0, glycan_mass); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calculated values are changed to heuristics and vice versa. Is this okay? Do we have tests for glycan database reading?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fragment is for N-glycan fragmentation, we don't use that in O-pair search right now. But, I still do a small revision to correct the fragment issue. A tester is also built.
private readonly int OxoniumIon204Index = 9; //Check Glycan.AllOxoniumIons | ||
protected readonly List<GlycoSpectralMatch>[] GlobalCsms; | ||
private readonly int OxoniumIon204Index = 9; // Check Glycan.AllOxoniumIons | ||
protected readonly List<GlycoSpectralMatch>[] GlobalCsms; // Why don't we call it GlobalGsms? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should make this change to Gsms. They likely did it because they copied the structure from cross link search
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. I already changed it.
public static List<Tuple<int, int, bool>> GetLocalizedGlycan(List<Route> OGlycanBoxLocalization, out LocalizationLevel localizationLevel) | ||
{ | ||
List<Tuple<int, int, bool>> localizedGlycan = new List<Tuple<int, int, bool>>(); | ||
|
||
//Dictionary<string, int>: modsite-id, count | ||
Dictionary<string, int> seenModSite = new Dictionary<string, int>(); | ||
Dictionary<string, int> ModSiteSeenCount = new Dictionary<string, int>(); // all possible glycan-sites pair, Dictionary<string, int>: site-glycan pair, count |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ModSiteSeenCount should be lowercase as it is accessible only inside this function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. Corrected it.
Slove the Nic's comment
1. Delete the Bullfrog glycan database (data from collaborator) 2. add the summary comment on the "WriteProteinGlycoLocalization" function
…com/RayMSMS/MetaMorpheus into Kdn-Adding-and-diagnostic-filtering
If we find 274/292 peak in Scan, we remove the glycan without A(NeuAc) in the candidate list.