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

Allow changes to molecule name (or anything that doesn't change its mass) without losing the association to any existing chromatograms. #2570

2 changes: 1 addition & 1 deletion pwiz_tools/Skyline/Controls/Graphs/GraphChromatogram.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2379,7 +2379,7 @@ private class DisplayPeptide

if (nodePep != null && nodeTranGroup != null)
{
var libKey = new LibKey(nodePep.ModifiedTarget, nodeTranGroup.PrecursorAdduct);
var libKey = new LibKey(nodePep.ChromatogramTarget, nodeTranGroup.PrecursorAdduct);
chromGraphPrimary.MidasRetentionMsMs = settings.PeptideSettings.Libraries.MidasLibraries
.SelectMany(lib => lib.GetSpectraByPeptide(chromGraphPrimary.Chromatogram.FilePath, libKey))
.Select(s => s.RetentionTime).ToArray();
Expand Down
2 changes: 1 addition & 1 deletion pwiz_tools/Skyline/Controls/Graphs/GraphSpectrum.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1358,7 +1358,7 @@ private void DoUpdate()

_graphHelper.ZoomSpectrumToSettings(DocumentUI, selection.NodeTranGroup);

if (GraphItem.PeptideDocNode != null && GraphItem.TransitionGroupNode != null && spectrum?.SpectrumInfo is SpectrumInfoLibrary libInfo)
if (GraphItem != null && GraphItem.PeptideDocNode != null && GraphItem.TransitionGroupNode != null && spectrum?.SpectrumInfo is SpectrumInfoLibrary libInfo)
{
var pepInfo = new ViewLibraryPepInfo(
GraphItem.PeptideDocNode.ModifiedTarget.GetLibKey(GraphItem.TransitionGroupNode.PrecursorAdduct),
Expand Down
15 changes: 13 additions & 2 deletions pwiz_tools/Skyline/Model/Peptide.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,16 @@ public Peptide(FastaSequence fastaSequence, string sequence, int? begin, int? en
Validate();
}

public Peptide(CustomMolecule customMolecule)
public Peptide(CustomMolecule customMolecule, Target originalMoleculeDescription = null)
{
Target = new Target(customMolecule);

Validate();
if (originalMoleculeDescription != null)
{
// As when user changes molecule name, CAS etc, but not mass or formula, so we still want to associate chromatograms
OriginalMoleculeTarget = originalMoleculeDescription;
}
}

public Peptide(Target pepOrMol)
Expand All @@ -84,6 +89,9 @@ public Peptide(Target pepOrMol)
public int Length { get { return Target.IsProteomic ? Target.Sequence.Length : 0; }}
public string TextId { get { return IsCustomMolecule ? Target.Molecule.InvariantName : Target.Sequence; } }

// For robust chromatogram association in the event of user tweaking molecule details like name, CAS etc (but not mass!)
public Target OriginalMoleculeTarget;

public SmallMoleculeLibraryAttributes GetSmallMoleculeLibraryAttributes()
{
return IsCustomMolecule ? CustomMolecule.GetSmallMoleculeLibraryAttributes() : SmallMoleculeLibraryAttributes.EMPTY;
Expand Down Expand Up @@ -340,6 +348,7 @@ private void Validate()
Assume.IsNull(_fastaSequence);
Assume.IsNull(Sequence);
CustomMolecule.Validate();
OriginalMoleculeTarget = Target;
}
else if (_fastaSequence == null)
{
Expand Down Expand Up @@ -380,6 +389,7 @@ public bool Equals(Peptide obj)
if (ReferenceEquals(this, obj)) return true;
var equal = Equals(obj._fastaSequence, _fastaSequence) &&
Equals(obj.Target, Target) &&
Equals(obj.OriginalMoleculeTarget, OriginalMoleculeTarget) &&
obj.Begin.Equals(Begin) &&
obj.End.Equals(End) &&
obj.MissedCleavages == MissedCleavages &&
Expand All @@ -400,7 +410,8 @@ public override int GetHashCode()
unchecked
{
int result = (_fastaSequence != null ? _fastaSequence.GetHashCode() : 0);
result = (result*397) ^ (Target != null ? Target.GetHashCode() : 0);
result = (result * 397) ^ (Target != null ? Target.GetHashCode() : 0);
result = (result * 397) ^ (OriginalMoleculeTarget != null ? OriginalMoleculeTarget.GetHashCode() : 0);
result = (result*397) ^ (Begin.HasValue ? Begin.Value : 0);
result = (result*397) ^ (End.HasValue ? End.Value : 0);
result = (result*397) ^ MissedCleavages;
Expand Down
8 changes: 7 additions & 1 deletion pwiz_tools/Skyline/Model/PeptideDocNode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,9 @@ public string GetCrosslinkedSequence()
public Color Color { get; private set; }
public static readonly Color UNKNOWN_COLOR = Color.FromArgb(170, 170, 170);

// For robust chromatogram association in the event of user tweaking molecule details like name, CAS etc (but not mass!)
public Target ChromatogramTarget => Peptide.OriginalMoleculeTarget ?? ModifiedTarget;

public Target Target { get { return Peptide.Target; }}
public string TextId { get { return CustomInvariantNameOrText(Peptide.Sequence); } }
public string RawTextId { get { return CustomInvariantNameOrText(ModifiedTarget.Sequence); } }
Expand Down Expand Up @@ -682,7 +685,10 @@ public PeptideDocNode ChangeExplicitRetentionTime(double? prop)
// Note: this potentially returns a node with a different ID, which has to be Inserted rather than Replaced
public PeptideDocNode ChangeCustomIonValues(SrmSettings settings, CustomMolecule customMolecule, ExplicitRetentionTimeInfo explicitRetentionTime)
{
var newPeptide = new Peptide(customMolecule);
// Make note of original description for chromatogram association, if nothing has changed to affect the chromatogram, so we can keep the association
var sameMass = Equals(customMolecule.AverageMass, Peptide.CustomMolecule.AverageMass) &&
Equals(customMolecule.Formula, Peptide.CustomMolecule.Formula);
var newPeptide = new Peptide(customMolecule, sameMass ? Peptide.Target : null);
Helpers.AssignIfEquals(ref newPeptide, Peptide);
if (Equals(Peptide, newPeptide))
{
Expand Down
2 changes: 1 addition & 1 deletion pwiz_tools/Skyline/Model/Results/ChromDataSet.cs
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ public Target ModifiedSequence
var peptideDocNode = NodeGroups.FirstOrDefault()?.Item1;
if (peptideDocNode != null)
{
return peptideDocNode.ModifiedTarget;
return peptideDocNode.ChromatogramTarget;
}
}
return _listChromData.Count > 0 ? BestChromatogram.Key.Target : null;
Expand Down
2 changes: 1 addition & 1 deletion pwiz_tools/Skyline/Model/Results/ChromatogramCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ private bool TextIdEqual(ChromGroupHeaderInfo entry, PeptideDocNode nodePep)
int textIdLen = entry.TextIdLen;
if (nodePep.Peptide.IsCustomMolecule)
{
if (EqualTextIdBytes(nodePep.CustomMolecule.ToSerializableString(), textIdIndex, textIdLen))
if (EqualTextIdBytes(nodePep.ChromatogramTarget.ToSerializableString(), textIdIndex, textIdLen))
{
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ internal PeptideChromDataSets MakePeptideChromDataSets()
continue;
}
var rawTimeIntensities = chromatogramInfo.TimeIntensities;
var chromKey = new ChromKey(PeptideDocNode.ModifiedTarget, transitionGroup.PrecursorMz, null,
var chromKey = new ChromKey(PeptideDocNode.ChromatogramTarget, transitionGroup.PrecursorMz, null,
transition.Mz, 0, 0, 0, transition.IsMs1 ? ChromSource.ms1 : ChromSource.fragment,
ChromExtractor.summed, true, false);
chromDatas.Add(new ChromData(chromKey, transition, rawTimeIntensities, rawTimeIntensities));
Expand Down
2 changes: 1 addition & 1 deletion pwiz_tools/Skyline/Model/Results/PeptideChromData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ public bool Load(ChromDataProvider provider)
foreach (var set in _dataSets.ToArray())
{
Color peptideColor = NodePep != null ? NodePep.Color : PeptideDocNode.UNKNOWN_COLOR;
if (!set.Load(provider, NodePep != null ? NodePep.ModifiedTarget : null, peptideColor))
if (!set.Load(provider, NodePep != null ? NodePep.ChromatogramTarget : null, peptideColor))
_dataSets.Remove(set);
}
//Console.Out.WriteLine("Ending {0} {1} {2}", NodePep, _dataSets.Count, RuntimeHelpers.GetHashCode(this));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ private void ExtractChromatogramsLocked()
var peptideNode = peptideFinder != null ? peptideFinder.FindPeptide(precursorMz) : null;
ProcessSrmSpectrum(
(float) dataSpectrum.RetentionTime.Value,
peptideNode != null ? peptideNode.ModifiedTarget : null,
peptideNode != null ? peptideNode.ChromatogramTarget : null,
peptideNode != null ? peptideNode.Color : PeptideDocNode.UNKNOWN_COLOR,
precursorMz,
filterIndex,
Expand Down
2 changes: 1 addition & 1 deletion pwiz_tools/Skyline/Model/Results/SpectrumFilter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ public sealed class SpectrumFilter : IFilterInstrumentInfo
var ce = step.HasValue
? document.GetCollisionEnergy(nodePep, nodeGroup, null, step.Value)
: (double?)null;
var key = new PrecursorTextId(mz, step, ce, ionMobilityFilter, nodePep.ModifiedTarget, ChromExtractor.summed);
var key = new PrecursorTextId(mz, step, ce, ionMobilityFilter, nodePep.ChromatogramTarget, ChromExtractor.summed);

if (!dictPrecursorMzToFilter.TryGetValue(key, out var filter))
{
Expand Down
9 changes: 8 additions & 1 deletion pwiz_tools/Skyline/Model/Serialization/DocumentReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1013,6 +1013,7 @@ private PeptideDocNode ReadPeptideXml(XmlReader reader, PeptideGroup group, bool
TransitionGroupDocNode[] children = null;
Adduct adduct = Adduct.EMPTY;
var customMolecule = isCustomMolecule ? CustomMolecule.Deserialize(reader, out adduct) : null; // This Deserialize only reads attributes, doesn't advance the reader
Target chromatogramTarget = null;
if (customMolecule != null)
{
if (DocumentMayContainMoleculesWithEmbeddedIons && customMolecule.ParsedMolecule.IsMassOnly && customMolecule.MonoisotopicMass.IsMassH())
Expand All @@ -1023,10 +1024,16 @@ private PeptideDocNode ReadPeptideXml(XmlReader reader, PeptideGroup group, bool
customMolecule.AverageMass.ChangeIsMassH(false),
customMolecule.Name);
}
// If user changed any molecule details (other than formula or mass) after chromatogram extraction, this info continues the target->chromatogram association
var encodedChromatogramTarget = reader.GetAttribute(ATTR.chromatogram_target);
if (!string.IsNullOrEmpty(encodedChromatogramTarget))
{
chromatogramTarget = Target.FromSerializableString(encodedChromatogramTarget);
}
}
Assume.IsTrue(DocumentMayContainMoleculesWithEmbeddedIons || adduct.IsEmpty); // Shouldn't be any charge info at the peptide/molecule level
var peptide = isCustomMolecule ?
new Peptide(customMolecule) :
new Peptide(customMolecule, chromatogramTarget) :
new Peptide(group as FastaSequence, sequence, start, end, missedCleavages, isDecoy);
if (reader.IsEmptyElement)
reader.Read();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ public static class ATTR
public const string precursor_concentration = "precursor_concentration";
public const string attribute_group_id = "attribute_group_id";
public const string peptide_index = "peptide_index";
public const string chromatogram_target = "chromatogram_target";

// Results
public const string replicate = "replicate";
Expand Down
5 changes: 5 additions & 0 deletions pwiz_tools/Skyline/Model/Serialization/DocumentWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,11 @@ private void WritePeptideXml(XmlWriter writer, PeptideDocNode node)
if (isCustomIon)
{
peptide.CustomMolecule.WriteXml(writer, Adduct.EMPTY);
// If user changed any molecule details (other than formula or mass) after chromatogram extraction, this info continues the target->chromatogram association
if (!Equals(peptide.Target, peptide.OriginalMoleculeTarget))
{
writer.WriteAttributeString(ATTR.chromatogram_target, peptide.OriginalMoleculeTarget.ToSerializableString());
}
}
else
{
Expand Down
11 changes: 11 additions & 0 deletions pwiz_tools/Skyline/TestFunctional/AgilentGCEITest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/

using Microsoft.VisualStudio.TestTools.UnitTesting;
using pwiz.Skyline.SettingsUI;
using pwiz.SkylineTestUtil;

namespace pwiz.SkylineTestFunctional
Expand Down Expand Up @@ -51,6 +52,16 @@ protected override void DoTest()
// Extract chromatograms from an Agilent file describing GC EI data as MS1
ImportResults(new[] { TestFilesDir.GetTestPath(@"cmsptc_00000_20230106_SCFA_1.mzML") });

// Now tinker with molecule details that don't change the molecule mass - should still be able to associate chromatograms
RunUI(() => { SkylineWindow.SequenceTree.SelectedNode = SkylineWindow.SequenceTree.Nodes[0].FirstNode; });
var doc = SkylineWindow.Document;
var editMoleculeDlg = ShowDialog<EditCustomMoleculeDlg>(SkylineWindow.ModifyPeptide);
RunUI(() => { editMoleculeDlg.NameText = editMoleculeDlg.NameText + "_renamed"; }); // Change the name - will not break the molecule/chromatogram association
// RunUI(() => { editMoleculeDlg.FormulaBox.Formula += "N2"; }); This would break the molecule/chromatogram association
OkDialog(editMoleculeDlg, editMoleculeDlg.OkDialog);
WaitForDocumentChangeLoaded(doc);
VerifyAllTransitionsHaveChromatograms(); // Verify that name change did not break the molecule/chromatogram association

// If data has not been properly understood as GC EI all-ions, some peaks won't be found
var nPeaksFound = 0;
foreach (var tg in SkylineWindow.Document.MoleculeTransitions)
Expand Down
44 changes: 19 additions & 25 deletions pwiz_tools/Skyline/TestFunctional/SrmSmMolChromTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
* limitations under the License.
*/
using Microsoft.VisualStudio.TestTools.UnitTesting;
using pwiz.Skyline.Model;
using pwiz.Skyline.Model.Results;
using pwiz.Skyline.SettingsUI;
using pwiz.SkylineTestUtil;

namespace pwiz.SkylineTestFunctional
Expand All @@ -39,36 +39,30 @@ public void TestSrmSmallMoleculeChromatograms()

protected override void DoTest()
{
RunUI(() => SkylineWindow.OpenFile(TestFilesDir.GetTestPath("SrmSmMolChromTest.sky")));
var testPath = TestFilesDir.GetTestPath("SrmSmMolChromTest.sky");
RunUI(() => SkylineWindow.OpenFile(testPath));
ImportResultsFiles(new[]
{
new MsDataFilePath(TestFilesDir.GetTestPath("ID36310_01_WAA263_3771_030118" + ExtensionTestContext.ExtMz5)),
new MsDataFilePath(TestFilesDir.GetTestPath("ID36311_01_WAA263_3771_030118" + ExtensionTestContext.ExtMz5))
});
VerifyAllTransitionsHaveChromatograms(SkylineWindow.Document);
}
VerifyAllTransitionsHaveChromatograms();

private void VerifyAllTransitionsHaveChromatograms(SrmDocument doc)
{
var tolerance = (float) doc.Settings.TransitionSettings.Instrument.MzMatchTolerance;
foreach (var molecule in doc.Molecules)
{
foreach (var precursor in molecule.TransitionGroups)
{
foreach (var chromatogramSet in doc.MeasuredResults.Chromatograms)
{
ChromatogramGroupInfo[] chromatogramGroups;
Assert.IsTrue(doc.MeasuredResults.TryLoadChromatogram(chromatogramSet, molecule, precursor, tolerance, out chromatogramGroups));
Assert.AreEqual(1, chromatogramGroups.Length);
foreach (var transition in precursor.Transitions)
{
var chromatogram = chromatogramGroups[0].GetTransitionInfo(transition, tolerance);
Assert.IsNotNull(chromatogram);
}
}
}
}

// Now tinker with molecule details that don't change the molecule mass - should still be able to associate chromatograms
RunUI(() => { SkylineWindow.SequenceTree.SelectedNode = SkylineWindow.SequenceTree.Nodes[0].FirstNode; });
var doc = SkylineWindow.Document;
var editMoleculeDlg = ShowDialog<EditCustomMoleculeDlg>(SkylineWindow.ModifyPeptide);
RunUI(() => { editMoleculeDlg.NameText = editMoleculeDlg.NameText + "_renamed"; }); // Change the name
OkDialog(editMoleculeDlg, editMoleculeDlg.OkDialog);
WaitForDocumentChangeLoaded(doc);
VerifyAllTransitionsHaveChromatograms();

// That info should survive serialization round trip
RunUI(() => SkylineWindow.SaveDocument(testPath));
RunUI(() => SkylineWindow.NewDocument());
RunUI(() => SkylineWindow.OpenFile(testPath));
VerifyAllTransitionsHaveChromatograms();
}

}
}
2 changes: 2 additions & 0 deletions pwiz_tools/Skyline/TestUtil/Schemas/Skyline_Current.xsd
Original file line number Diff line number Diff line change
Expand Up @@ -921,6 +921,8 @@
<xs:attribute name="custom_ion_name" type="xs:string" use="optional"/>
<!--Any or all of InChiKey, CAS, HMDB etc tab separated -->
<xs:attribute name="id" type="xs:string" use="optional"/>
<!-- Encodes the molecule description used in chromatogram extraction if user changed the name etc afterwards -->
<xs:attribute name="chromatogram_target" type="xs:string" use="optional"/>
</xs:extension>
</xs:complexContent>
</xs:complexType>
Expand Down
23 changes: 23 additions & 0 deletions pwiz_tools/Skyline/TestUtil/TestFunctional.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2521,6 +2521,29 @@ public void ImportResultsFiles(string dirPath, string ext, string filter, bool?
WaitForDocumentChange(doc);
}

public void VerifyAllTransitionsHaveChromatograms()
{
var doc = WaitForDocumentLoaded();
var tolerance = (float)doc.Settings.TransitionSettings.Instrument.MzMatchTolerance;
foreach (var molecule in doc.Molecules)
{
foreach (var precursor in molecule.TransitionGroups)
{
foreach (var chromatogramSet in doc.MeasuredResults.Chromatograms)
{
ChromatogramGroupInfo[] chromatogramGroups;
Assert.IsTrue(doc.MeasuredResults.TryLoadChromatogram(chromatogramSet, molecule, precursor, tolerance, out chromatogramGroups));
Assert.AreEqual(1, chromatogramGroups.Length);
foreach (var transition in precursor.Transitions)
{
var chromatogram = chromatogramGroups[0].GetTransitionInfo(transition, tolerance);
Assert.IsNotNull(chromatogram);
}
}
}
}
}

#endregion

#region Spectral library test helpers
Expand Down