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 new SkylineCmd arguments for building ion mobility libraries: -… #2301

Merged
merged 16 commits into from Oct 4, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
2c71cff
Added new SkylineCmd arguments for building ion mobility libraries: -…
bspratt Sep 23, 2022
3721ec9
Changes from Brendan's code review:
bspratt Sep 23, 2022
34e4936
Extend this test to include an expected failure, and use of optional …
bspratt Sep 26, 2022
4aa851f
Add error handling for trying to create an imsdb file in a nonexisten…
bspratt Sep 27, 2022
cb68a1a
Merge branch 'master' into Skyline/work/20220923_automate_imsb_creation
bspratt Sep 27, 2022
cc1ffc0
Merge branch 'master' into Skyline/work/20220923_automate_imsb_creation
bspratt Sep 29, 2022
9348883
Merge branch 'Skyline/work/20220923_automate_imsb_creation' of https:…
bspratt Sep 29, 2022
d96b2fb
Centralize some code for comparing DSV files created with different c…
bspratt Sep 30, 2022
ac1419c
Merge branch 'master' into Skyline/work/20220923_automate_imsb_creation
bspratt Sep 30, 2022
a2d2824
previous commit broke writing .imsb to implied current working directory
bspratt Sep 30, 2022
346e5a3
Merge branch 'Skyline/work/20220923_automate_imsb_creation' of https:…
bspratt Sep 30, 2022
54213ee
- clean-up test with increased abstraction
brendanx67 Oct 2, 2022
0555062
More code cleanup from Brendan's review.
bspratt Oct 3, 2022
5e4175e
Merge branch 'master' into Skyline/work/20220923_automate_imsb_creation
bspratt Oct 3, 2022
b8af30c
- move DetermineDsvDelimiter to AssertEx as the best location for now
brendanx67 Oct 4, 2022
e0d1032
Merge branch 'master' into Skyline/work/20220923_automate_imsb_creation
brendanx67 Oct 4, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
27 changes: 27 additions & 0 deletions pwiz_tools/Skyline/CommandArgUsage.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions pwiz_tools/Skyline/CommandArgUsage.resx
Expand Up @@ -123,6 +123,12 @@
<data name="_add_library_path" xml:space="preserve">
<value>Specify a spectral library to be added to the open document.</value>
</data>
<data name="_ionmobility_create_library" xml:space="preserve">
<value>Add an ion mobility library to the open document, based on its currently loaded chromatograms.</value>
</data>
<data name="_ionmobility_create_library_name" xml:space="preserve">
<value>Name to give the ion mobility library in an –-ionmobility-create-library operation.</value>
</data>
<data name="_batch_commands" xml:space="preserve">
<value>Runs a file line by line treating each line like a SkylineRunner/Cmd input. Useful for automating the execution of multiple commands. The open Skyline file remains active through all commands.</value>
</data>
Expand Down Expand Up @@ -387,6 +393,9 @@
<data name="CommandArgs_GROUP_IMPORT_Importing_results_replicates" xml:space="preserve">
<value>Importing results replicates</value>
</data>
<data name="CommandArgs_GROUP_CREATE_IMSDB_Ion_Mobility_Library" xml:space="preserve">
<value>Creating an ion mobility library</value>
Copy link
Contributor

Choose a reason for hiding this comment

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

What about adding to an existing IMSDB? Seems like that should also be possible.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed, that just wasn't the immediate requirement. If you agree I'll just open an issue for that and return my attention to Feature Finding.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is at least worth thinking through now how you would support that within the command argument group you have created. Maybe making this title something more generic like:

Ion mobility library support

And arguments maybe:

--ionmobility-library-add=path/to/file.imsdb
--ionmobility-library-create=path/to/file.imsdb
--ionmobility-library-name=name

And eventually:

--ionmobility-library-addresults

create assumes addresults, obviously. Though, a user might use the combination:

--ionmobility-library-add=path/to/file.imsdb --ionmobility-library-name=name --ionmobility-library-addresults

which might ultimately be what Scripps wants to do to create a single .imsdb with all of their compounds in it for sharing with other Skyline users.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed, the hierarchy should be --ionmobility-library-XXX rather than --ionmobility-create-XXX for best extensibility.

Copy link
Member Author

Choose a reason for hiding this comment

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

image

</data>
<data name="CommandArgs_GROUP_REMOVE_Removing_results_replicates" xml:space="preserve">
<value>Removing results replicates</value>
</data>
Expand Down
23 changes: 23 additions & 0 deletions pwiz_tools/Skyline/CommandArgs.cs
Expand Up @@ -215,6 +215,17 @@ public bool Saving
public string SharedFile { get; private set; }
public ShareType SharedFileType { get; private set; }

// For creating a .imsdb ion mobility library
public static readonly Argument ARG_CREATE_IMSDB = new DocArgument(@"ionmobility-create-library", PATH_TO_FILE,
bspratt marked this conversation as resolved.
Show resolved Hide resolved
(c, p) => c.CreateIMSDB(p));
bspratt marked this conversation as resolved.
Show resolved Hide resolved

// For creating a .imsdb ion mobility library
public static readonly Argument ARG_CREATE_IMSDB_NAME = new DocArgument(@"ionmobility-create-library-name", NAME_VALUE,
(c, p) => c.IMSDbName = p.Value);

private static readonly ArgumentGroup GROUP_CREATE_IMSDB = new ArgumentGroup(() => CommandArgUsage.CommandArgs_GROUP_CREATE_IMSDB_Ion_Mobility_Library, false,
brendanx67 marked this conversation as resolved.
Show resolved Hide resolved
ARG_CREATE_IMSDB, ARG_CREATE_IMSDB_NAME);

public static readonly Argument ARG_IMPORT_FILE = new DocArgument(@"import-file", PATH_TO_FILE,
(c, p) => c.ParseImportFile(p));
public static readonly Argument ARG_IMPORT_REPLICATE_NAME = new DocArgument(@"import-replicate-name", NAME_VALUE,
Expand Down Expand Up @@ -337,6 +348,8 @@ private bool ValidateMinimizeResultsArgs()
return true;
}

public MsDataFileUri IMSDbFile { get; private set; }
Copy link
Contributor

Choose a reason for hiding this comment

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

Too many capitals in the naming. Use ImsDb or Imsdb instead, like MsDataFile. We prefer title-case even for acronyms.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, will fix

public string IMSDbName { get; private set; }

public List<MsDataFileUri> ReplicateFile { get; private set; }
public string ReplicateName { get; private set; }
Expand All @@ -362,6 +375,11 @@ private bool ValidateMinimizeResultsArgs()
public double? LockmassTolerance { get; private set; }
public LockMassParameters LockMassParameters { get { return new LockMassParameters(LockmassPositive, LockmassNegative, LockmassTolerance); } }

private void CreateIMSDB(NameValuePair pair)
{
IMSDbFile = new MsDataFilePath(pair.ValueFullPath);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not an MsDataFile so strange to be using MsDataFilePath.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, I'll fix that, thanks.

}

private void ParseImportFile(NameValuePair pair)
{
ReplicateFile.Add(new MsDataFilePath(pair.ValueFullPath));
Expand Down Expand Up @@ -556,6 +574,10 @@ public bool AddDecoys
{
get { return !string.IsNullOrEmpty(AddDecoysType); }
}
public bool CreatingIMSDB
{
get { return IMSDbFile != null; }
}
public bool ImportingResults
{
get { return ImportingReplicateFile || ImportingSourceDirectory; }
Expand Down Expand Up @@ -1784,6 +1806,7 @@ public static IEnumerable<IUsageBlock> UsageBlocks
GROUP_IMPORT_SEARCH,
GROUP_IMPORT_LIST,
GROUP_ADD_LIBRARY,
GROUP_CREATE_IMSDB,
GROUP_DECOYS,
GROUP_REFINEMENT,
GROUP_REFINEMENT_W_RESULTS,
Expand Down
41 changes: 41 additions & 0 deletions pwiz_tools/Skyline/CommandLine.cs
Expand Up @@ -376,6 +376,11 @@ private bool ProcessDocument(CommandArgs commandArgs)
return false;
}

if (!CreateIMSDB(commandArgs))
bspratt marked this conversation as resolved.
Show resolved Hide resolved
{
return false;
}

if (commandArgs.Saving)
{
var saveFile = commandArgs.SaveFile ?? _skylineFile;
Expand Down Expand Up @@ -588,6 +593,42 @@ private bool RefineDocument(CommandArgs commandArgs)
}
}

private bool CreateIMSDB(CommandArgs commandArgs)
{
if (commandArgs.IMSDbFile == null)
{
return true; // Nothing to do
}
_out.WriteLine(Resources.CommandLine_CreateIMSDB_Creating_an_ion_mobility_library___);
bspratt marked this conversation as resolved.
Show resolved Hide resolved
try
{
ModifyDocumentWithLogging(doc => doc.ChangeSettings(doc.Settings.ChangeTransitionSettings(p =>
bspratt marked this conversation as resolved.
Show resolved Hide resolved
{
var progressMonitor = new CommandProgressMonitor(_out, new ProgressStatus(string.Empty));
var path = commandArgs.IMSDbFile.GetFilePath();
var name = commandArgs.IMSDbName ?? Path.GetFileNameWithoutExtension(path);
var lib = IonMobilityLibrary.CreateFromResults(
doc, null, false, name, path,
progressMonitor);

return p.ChangeIonMobilityFiltering(p.IonMobilityFiltering.ChangeLibrary(lib));
})), AuditLogEntry.SettingsLogFunction);
return true;
}
catch (Exception x)
{
if (!_out.IsErrorReported)
{
_out.WriteLine(Resources.CommandLine_GeneralException_Error___0_, x.Message);
}
else
{
_out.WriteLine(x.Message);
}
return false;
}
}

private IsotopeLabelType GetLabelTypeHelper(string label)
{
var mods = _doc.Settings.PeptideSettings.Modifications;
Expand Down
9 changes: 9 additions & 0 deletions pwiz_tools/Skyline/Properties/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions pwiz_tools/Skyline/Properties/Resources.resx
Expand Up @@ -11838,4 +11838,7 @@ If you do not have the original file, you may build the library with embedded sp
<data name="SkylineWindow_submitErrorReportMenuItem_Click_Submitting_an_unhandled_error_report" xml:space="preserve">
<value>Submitting an unhandled error report</value>
</data>
<data name="CommandLine_CreateIMSDB_Creating_an_ion_mobility_library___" xml:space="preserve">
<value>Creating an ion mobility library...</value>
</data>
</root>
10 changes: 8 additions & 2 deletions pwiz_tools/Skyline/TestPerf/PerfMeasuredInverseK0.cs
Expand Up @@ -200,18 +200,24 @@ private void TestCommandlineImport(string skyFile, string testPath)
{
if (File.Exists(testPath))
File.Delete(testPath);
var imsdbPath = TestFilesDir.GetTestPath("testlib.imsdb");

// Show anyone watching that work is being performed
// Create an ion mobility library after import just to exercise that code
RunUI(() => {
using (var longWait = new LongWaitDlg(SkylineWindow) { Message = "Running command-line import" })
{
longWait.PerformWork(SkylineWindow, 500, () =>
{
RunCommand("--in=" + skyFile,
"--import-file=" + TestFilesDir.GetTestPath(BSA_50fmol_TIMS_InfusionESI_10precd),
"--out=" + testPath));
"--ionmobility-create-library=" + imsdbPath,
bspratt marked this conversation as resolved.
Show resolved Hide resolved
"--ionmobility-create-library-name=testlib",
"--out=" + testPath);
});
}
});

AssertEx.FileExists(imsdbPath);
VerifySerialization(testPath, false);
}

Expand Down