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.

Sign up for GitHub

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

Skyline: Remove BuildLibraryNotification and switch to LongWaitDlg #3350

Merged
merged 4 commits into from
Jan 28, 2025
Merged

Skyline: Remove BuildLibraryNotification and switch to LongWaitDlg #3350

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
004e05d
Skyline: Remove BuildLibraryNotification and switch to LongWaitDlg
brendanx67 Jan 27, 2025
56515de
- Move code originally in BuildLibraryNotification from SkylineWindow…
brendanx67 Jan 28, 2025
b8dc28b
- respond to Nick's feedback and fix TeamCity failures
brendanx67 Jan 28, 2025
819ab10
Merge branch 'master' into Skyline/work/20250124_remove_build_lib_not…
brendanx67 Jan 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion pwiz_tools/Skyline/CommandLine.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2323,6 +2323,7 @@ private IrtStandard GetIrtStandard(CommandArgs commandArgs)
Equals(standard.Name, commandArgs.IrtStandardName));
if (irtStandard == null)
{
// TODO: This should really be an Error that causes processing to stop rather than information treated like no iRT standard was specified
_out.WriteLine(SkylineResources.CommandLine_ImportSearchInternal_The_iRT_standard_name___0___is_invalid_,
commandArgs.IrtStandardName);
return null;
Expand Down Expand Up @@ -2351,7 +2352,10 @@ private bool ImportSearchInternal(CommandArgs commandArgs, ref SrmDocument doc)
foreach (var file in commandArgs.SearchResultsFiles)
_out.WriteLine(Path.GetFileName(file));
if (!builder.BuildLibrary(progressMonitor))
{
_out.WriteLine(SkylineResources.CommandLine_ImportSearchInternal_Error__Failed_to_build_the_spectral_library_);
return false;
}

if (!string.IsNullOrEmpty(builder.AmbiguousMatchesMessage))
_out.WriteLine(builder.AmbiguousMatchesMessage);
Expand Down Expand Up @@ -2393,7 +2397,7 @@ private bool ImportSearchInternal(CommandArgs commandArgs, ref SrmDocument doc)
import.IrtStandard = autoStandards[0];
break;
default:
_out.WriteLine(SkylineResources.CommandLine_ImportSearchInternal_iRT_standard_set_to__0___but_multiple_iRT_standards_were_found__iRT_standard_must_be_set_explicitly_,
_out.WriteLine(SkylineResources.CommandLine_ImportSearchInternal_Error__iRT_standard_set_to__0___but_multiple_iRT_standards_were_found__iRT_standard_must_be_set_explicitly_,
IrtStandard.AUTO.Name);
return false;
}
Expand Down
20 changes: 15 additions & 5 deletions pwiz_tools/Skyline/FileUI/PeptideSearch/BuildPeptideSearchLibraryControl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ private bool BuildPeptideSearchLibrary(CancelEventArgs e, bool showWarnings, boo
return false;
}

bool retry = false;
bool retry;
do
{
using (var longWaitDlg = new LongWaitDlg())
Expand Down Expand Up @@ -379,10 +379,11 @@ private bool BuildPeptideSearchLibrary(CancelEventArgs e, bool showWarnings, boo
var response = ShowLibraryMissingExternalSpectraError(WizardForm, status.ErrorException);
if (response == UpdateProgressResponse.cancel)
return false;
else if (response == UpdateProgressResponse.normal)
builder.PreferEmbeddedSpectra = true;

builder.PreferEmbeddedSpectra = response == UpdateProgressResponse.normal;

retry = true;
continue;
}
else
{
Expand All @@ -398,7 +399,10 @@ private bool BuildPeptideSearchLibrary(CancelEventArgs e, bool showWarnings, boo
return false;
}
}
} while (retry) ;

retry = false;

} while (retry);

var docLibSpec = builder.LibrarySpec;
BuildLibrarySettings.LibraryName = docLibSpec.Name;
Expand All @@ -418,7 +422,7 @@ private bool BuildPeptideSearchLibrary(CancelEventArgs e, bool showWarnings, boo
return false;

IrtStandard outStandard = null;
var addedIrts = !isFeatureDetection && LibraryBuildNotificationHandler.AddIrts(IrtRegressionType.DEFAULT,
var addedIrts = !isFeatureDetection && PeptideSettingsUI.AddIrts(IrtRegressionType.DEFAULT,
ImportPeptideSearch.DocLib, docLibSpec, _driverStandards.SelectedItem, WizardForm, false, out outStandard);

var docNew = ImportPeptideSearch.AddDocumentSpectralLibrary(DocumentContainer.Document, docLibSpec);
Expand Down Expand Up @@ -477,6 +481,12 @@ private bool AddExistingLibrary(CancelEventArgs e)
return false;
}

if (DocumentContainer.Document.Settings.PeptideSettings.Libraries.LibrarySpecs.Contains(docLibSpec))
{
// No further work necessary.
return true;
}

var docNew = ImportPeptideSearch.AddDocumentSpectralLibrary(DocumentContainer.Document, docLibSpec);
if (docNew == null)
return false;
Expand Down
24 changes: 17 additions & 7 deletions pwiz_tools/Skyline/FileUI/PeptideSearch/ImportPeptideSearchDlg.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -567,22 +567,32 @@ private void NextPage()
{
if (!BuildPepSearchLibControl.PerformDDASearch)
{
HasPeakBoundaries = BuildPepSearchLibControl.SearchFilenames.All(f => f.EndsWith(BiblioSpecLiteBuilder.EXT_TSV));
if (BuildPepSearchLibControl.SearchFilenames.Any(f => f.EndsWith(BiblioSpecLiteBuilder.EXT_TSV)) && !HasPeakBoundaries)
{
MessageDlg.Show(this, PeptideSearchResources.ImportPeptideSearchDlg_NextPage_Cannot_build_library_from_OpenSWATH_results_mixed_with_results_from_other_tools_);
return;
}
HasPeakBoundaries = BuildPepSearchLibControl.SearchFilenames.All(f =>
f.EndsWith(BiblioSpecLiteBuilder.EXT_TSV));
if (BuildPepSearchLibControl.SearchFilenames.Any(f =>
f.EndsWith(BiblioSpecLiteBuilder.EXT_TSV)) && !HasPeakBoundaries)
{
MessageDlg.Show(this,
PeptideSearchResources
.ImportPeptideSearchDlg_NextPage_Cannot_build_library_from_OpenSWATH_results_mixed_with_results_from_other_tools_);
return;
}
}
}

var eCancel = new CancelEventArgs();
if (!BuildPepSearchLibControl.PerformDDASearch && !BuildPeptideSearchLibrary(eCancel, IsFeatureDetectionWorkflow))
{
// Page shows error
if (eCancel.Cancel)
{
// Page has shown an error and canceled further progress
return;
}
// A failure has occurred
// CONSIDER: Should this ever cause the wizard to be closed and force the user to start over?
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand the "Should this ever cause the wizard to be closed" part of this comment. It looks like it always causes the wizard to be closed. Is there a typo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Think of CONSIDER as not quite as forceful as a TODO. This code maybe should be changed, but I am leaving it as it was when I found it. I am not sure why we want to close the wizard. That may always be something a user would prefer we not do. But this code existed this way before. I am not changing the CloseWizard part. Just saying it should be reconsidered in the future.

CloseWizard(DialogResult.Cancel);
// Not a good idea to continue once the wizard has been closed
return;
}

// The user had the option to finish right after
Expand Down
71 changes: 29 additions & 42 deletions pwiz_tools/Skyline/Model/Lib/BiblioSpecLiteBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,54 +160,41 @@ public bool BuildLibrary(IProgressMonitor progress)
Charges = Charges,
};

bool retry = true;
do
try
{
try
if (!blibBuilder.BuildLibrary(Action, progress, ref status,
out _buildCommandArgs, out _buildOutput, out _ambiguousMatches))
{
if (!blibBuilder.BuildLibrary(Action, progress, ref status,
out _buildCommandArgs, out _buildOutput, out _ambiguousMatches))
{
return false;
}

retry = false;
}
catch (IOException x)
{
if (IsLibraryMissingExternalSpectraError(x, out IList<string> spectrumFilenames, out IList<string> directoriesSearched, out string resultsFilepath))
{
// replace the relative path to the results file (e.g. msms.txt) with the absolute path
string fullResultsFilepath = InputFiles.SingleOrDefault(o => o.EndsWith(resultsFilepath)) ??
throw new InvalidDataException(@"no results filepath from BiblioSpec error message", x);

// TODO: this will break if BiblioSpec output is translated to other languages
string messageWithFullFilepath =
x.Message.Replace(@"search results file '" + resultsFilepath,
@"search results file '" + fullResultsFilepath);

var response =
progress.UpdateProgress(
status.ChangeErrorException(new IOException(messageWithFullFilepath, x)));
if (response == UpdateProgressResponse.cancel)
return false;
else if (response == UpdateProgressResponse.normal)
blibBuilder.PreferEmbeddedSpectra = true;
continue;
}

progress.UpdateProgress(status.ChangeErrorException(x));
return false;
}
catch (Exception x)
}
catch (IOException x)
{
if (IsLibraryMissingExternalSpectraError(x, out IList<string> spectrumFilenames, out IList<string> directoriesSearched, out string resultsFilepath))
{
Console.WriteLine(x.Message);
progress.UpdateProgress(status.ChangeErrorException(
new Exception(string.Format(LibResources.BiblioSpecLiteBuilder_BuildLibrary_Failed_trying_to_build_the_redundant_library__0__,
redundantLibrary), x)));
return false;
// replace the relative path to the results file (e.g. msms.txt) with the absolute path
string fullResultsFilepath = InputFiles.SingleOrDefault(o => o.EndsWith(resultsFilepath));
if (fullResultsFilepath == null)
throw new InvalidDataException(@"no results filepath from BiblioSpec error message", x);

// TODO: this will break if BiblioSpec output is translated to other languages
string messageWithFullFilepath =
x.Message.Replace(@"search results file '" + resultsFilepath,
@"search results file '" + fullResultsFilepath);
x = new IOException(messageWithFullFilepath, x);
}
} while (retry);

progress.UpdateProgress(status.ChangeErrorException(x));
return false;
}
catch (Exception x)
{
Console.WriteLine(x.Message);
progress.UpdateProgress(status.ChangeErrorException(
new Exception(string.Format(LibResources.BiblioSpecLiteBuilder_BuildLibrary_Failed_trying_to_build_the_redundant_library__0__,
redundantLibrary), x)));
return false;
}

var blibFilter = new BlibFilter();
status = new ProgressStatus(message);
Expand Down
111 changes: 0 additions & 111 deletions pwiz_tools/Skyline/SettingsUI/BuildLibraryNotification.Designer.cs
View file
Open in desktop

This file was deleted.

Loading