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

Added custom protease window in GUI #2352

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

zhuoxinshi
Copy link
Contributor

No description provided.

Copy link

codecov bot commented Jul 30, 2024

Codecov Report

Attention: Patch coverage is 88.88889% with 3 lines in your changes missing coverage. Please review.

Project coverage is 93.64%. Comparing base (401a5a9) to head (e50e20b).
Report is 1 commits behind head on master.

Files Patch % Lines
MetaMorpheus/EngineLayer/GlobalVariables.cs 88.88% 2 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2352      +/-   ##
==========================================
+ Coverage   93.62%   93.64%   +0.02%     
==========================================
  Files         140      140              
  Lines       21895    21923      +28     
  Branches     3005     3009       +4     
==========================================
+ Hits        20499    20530      +31     
+ Misses        937      936       -1     
+ Partials      459      457       -2     
Files Coverage Δ
MetaMorpheus/EngineLayer/GlobalVariables.cs 85.13% <88.88%> (+0.29%) ⬆️

... and 2 files with indirect coverage changes

Copy link
Member

@nbollis nbollis left a comment

Choose a reason for hiding this comment

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

Great start! I have a few suggestions and recommendations

@@ -14,6 +15,9 @@
using Omics.Modifications;
using TopDownProteomics;
using UsefulProteomicsDatabases;
using Omics.Digestion;
using static System.Net.Mime.MediaTypeNames;
using System.Collections;
Copy link
Member

Choose a reason for hiding this comment

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

Are all of these new namespaces necessary?

}
if (DigestionMotifTextBox.Text.Length == 0)
{
MessageBox.Show("Please specify the digestion motif(s) of the protease");
Copy link
Member

Choose a reason for hiding this comment

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

Should we check to see if the digestion motiff is a valid amino acid?

//load the default ProteaseDictionary
//string folderPath = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFiles);
//string path = ((!string.IsNullOrWhiteSpace(folderPath) && AppDomain.CurrentDomain.BaseDirectory.Contains(folderPath) && !AppDomain.CurrentDomain.BaseDirectory.Contains("Jenkins")) ? System.IO.Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), "MetaMorpheus") : AppDomain.CurrentDomain.BaseDirectory);
//string path2 = System.IO.Path.Combine(path, "ProteolyticDigestion", "proteases.tsv");
Copy link
Member

Choose a reason for hiding this comment

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

comments can be removed

CleavageSpecDic.Add(c.ToString(), c);
}
CleavageSpecificity cleavageSpecificity = CleavageSpecDic[CleavageSpecComboBox.Text];
List<DigestionMotif> digestionMotifs;
Copy link
Member

Choose a reason for hiding this comment

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

Replace this dictionary creation with
if (Enum.TryParse(CleavageSpecComboBox.Text, out _ cleaveageSpec)
{
set the value
}
else
{
message box error and return
}

ProteaseDictionary.Dictionary.Add(proteaseName, proteaseToAdd);
}

if (!ProteaseDictionary.Dictionary.TryAdd(proteaseName, proteaseToAdd))
Copy link
Member

Choose a reason for hiding this comment

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

This check seems redundant

File.Create(customProteasePath);
}

string lineToAdd = "\n" + proteaseName + "\t" + motifString + "\t\t\t" + CleavageSpecComboBox.Text + "\t" + psiMsAccessionNum + "\t" + psiMsName + "\t\t\t\t\t";
Copy link
Member

Choose a reason for hiding this comment

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

what pieces of information are being skipped over with all of the \t\t\t\t and is that information important here?

DialogResult = false;
}

private void CleavageSpecComboBox_SelectionChanged(object sender, SelectionChangedEventArgs e)
Copy link
Member

Choose a reason for hiding this comment

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

unused method can be removed

{
string[] proteaseToAdd = File.ReadAllLines(customProteasePath);
for (int i = 0; i < proteaseToAdd.Length; i++)
{
Copy link
Member

Choose a reason for hiding this comment

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

Most likely need to start at i == 1 because the first row is the header

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