-
Notifications
You must be signed in to change notification settings - Fork 41
Isobaric Quant FileInfo and PlexAnnotation to achieve something analogous to SpectraFileInfo #990
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
base: master
Are you sure you want to change the base?
Changes from 2 commits
661f20c
150ce79
bf95be9
f34bfd5
910a349
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,120 @@ | ||||||||||||||||
| using System; | ||||||||||||||||
| using System.Collections.Generic; | ||||||||||||||||
| using System.IO; | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| namespace MassSpectrometry | ||||||||||||||||
| { | ||||||||||||||||
| /// <summary> | ||||||||||||||||
| /// Describes an isobaric-quantification input file and the plex annotations for that file. | ||||||||||||||||
| /// Immutable for its core identity (path, plex, fraction, technical replicate and annotations). | ||||||||||||||||
| /// </summary> | ||||||||||||||||
| public class IsobaricQuantFileInfo | ||||||||||||||||
| { | ||||||||||||||||
| /// <summary> | ||||||||||||||||
| /// Create a new instance describing a single isobaric quantification file. | ||||||||||||||||
| /// </summary> | ||||||||||||||||
| /// <param name="fullFilePathWithExtension">Absolute or relative path to the spectra/quant file including extension.</param> | ||||||||||||||||
| /// <param name="plex">The plex identifier (e.g., "TMT10", "TMTpro16") for this file. Null will be normalized to empty string.</param> | ||||||||||||||||
| /// <param name="fraction">Fraction index (1-based) for fractionated experiments.</param> | ||||||||||||||||
| /// <param name="technicalReplicate">Technical replicate index (1-based) for repeated injections/runs.</param> | ||||||||||||||||
| /// <param name="annotations">Ordered list of channel annotations for the file's plex (may be empty).</param> | ||||||||||||||||
| public IsobaricQuantFileInfo(string fullFilePathWithExtension, string plex, int fraction, int technicalReplicate, IReadOnlyList<IsobaricQuantPlexAnnotation> annotations) | ||||||||||||||||
| { | ||||||||||||||||
| FullFilePathWithExtension = fullFilePathWithExtension; | ||||||||||||||||
| Plex = plex ?? string.Empty; | ||||||||||||||||
| Fraction = fraction; | ||||||||||||||||
| TechnicalReplicate = technicalReplicate; | ||||||||||||||||
| Annotations = annotations ?? Array.Empty<IsobaricQuantPlexAnnotation>(); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| /// <summary> | ||||||||||||||||
| /// The path to the data file (for example a .raw or .mzML) including the extension. | ||||||||||||||||
| /// Used as the primary identity for equality and hashing. | ||||||||||||||||
| /// </summary> | ||||||||||||||||
| public string FullFilePathWithExtension { get; } | ||||||||||||||||
|
|
||||||||||||||||
| /// <summary> | ||||||||||||||||
| /// The plex identifier for this file (for example "TMT10" or "TMTpro16"). | ||||||||||||||||
| /// Empty string when not provided. | ||||||||||||||||
| /// </summary> | ||||||||||||||||
| public string Plex { get; } | ||||||||||||||||
|
|
||||||||||||||||
| /// <summary> | ||||||||||||||||
| /// Fraction index for the file. Uses 1-based indexing to match experimental notation. | ||||||||||||||||
| /// </summary> | ||||||||||||||||
| public int Fraction { get; } // 1-based | ||||||||||||||||
|
|
||||||||||||||||
| /// <summary> | ||||||||||||||||
| /// Technical replicate index for the file. Uses 1-based indexing. | ||||||||||||||||
| /// </summary> | ||||||||||||||||
| public int TechnicalReplicate { get; } // 1-based | ||||||||||||||||
|
|
||||||||||||||||
| /// <summary> | ||||||||||||||||
| /// All channel annotations for this file's plex (one entry per reporter/tag). | ||||||||||||||||
| /// Immutable reference to the provided read-only list (may be empty). | ||||||||||||||||
| /// </summary> | ||||||||||||||||
| public IReadOnlyList<IsobaricQuantPlexAnnotation> Annotations { get; } // All tags for this file's plex | ||||||||||||||||
|
|
||||||||||||||||
| /// <summary> | ||||||||||||||||
| /// Returns true when the other object represents the same file path. | ||||||||||||||||
| /// Equality is based on <see cref="FullFilePathWithExtension"/>. | ||||||||||||||||
| /// </summary> | ||||||||||||||||
| /// <param name="obj">Other object to compare.</param> | ||||||||||||||||
| /// <returns>True when <paramref name="obj"/> is an <see cref="IsobaricQuantFileInfo"/> with the same file path.</returns> | ||||||||||||||||
| public override bool Equals(object obj) | ||||||||||||||||
| { | ||||||||||||||||
| if (base.Equals(obj)) | ||||||||||||||||
| { | ||||||||||||||||
| return ((IsobaricQuantFileInfo)obj).FullFilePathWithExtension.Equals(FullFilePathWithExtension); | ||||||||||||||||
|
||||||||||||||||
| return ((IsobaricQuantFileInfo)obj).FullFilePathWithExtension.Equals(FullFilePathWithExtension); | |
| return ((IsobaricQuantFileInfo)obj).FullFilePathWithExtension.Equals(FullFilePathWithExtension, StringComparison.Ordinal); |
Copilot
AI
Dec 5, 2025
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 Equals method implementation is incorrect. It calls base.Equals(obj) first, which will only return true if both references point to the same object. This means the custom equality check on line 69 will never execute for different instances, even if they have the same file path.
The correct pattern (as seen in SpectraFileInfo) should be:
if (obj is not IsobaricQuantFileInfo other)
{
return false;
}
return FullFilePathWithExtension.Equals(other.FullFilePathWithExtension);Alternatively, use:
return obj is IsobaricQuantFileInfo other && FullFilePathWithExtension.Equals(other.FullFilePathWithExtension);| if (base.Equals(obj)) | |
| { | |
| return ((IsobaricQuantFileInfo)obj).FullFilePathWithExtension.Equals(FullFilePathWithExtension); | |
| } | |
| return false; | |
| return obj is IsobaricQuantFileInfo other && FullFilePathWithExtension.Equals(other.FullFilePathWithExtension); |
Copilot
AI
Dec 5, 2025
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 Equals method should be paired with an implementation of IEquatable<IsobaricQuantFileInfo> for better performance and type safety. This is a standard pattern for value objects that override Equals. Additionally, consider overriding the == and != operators for consistency with value object semantics.
Copilot
AI
Dec 5, 2025
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 GetHashCode() method will throw a NullReferenceException if FullFilePathWithExtension is null (which is possible given the constructor doesn't normalize null values).
For consistency with SpectraFileInfo, use:
return StringComparer.Ordinal.GetHashCode(FullFilePathWithExtension ?? string.Empty);Or alternatively, if the constructor is fixed to normalize nulls to empty strings (as suggested in another comment), this would be acceptable as-is.
| return FullFilePathWithExtension.GetHashCode(); | |
| return StringComparer.Ordinal.GetHashCode(FullFilePathWithExtension ?? string.Empty); |
Copilot
AI
Dec 5, 2025
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 ToString() method will throw a NullReferenceException if FullFilePathWithExtension is null (which is possible given the constructor doesn't normalize null values). Either fix the constructor to normalize nulls, or add null handling here:
return Path.GetFileName(FullFilePathWithExtension ?? string.Empty);| return Path.GetFileName(FullFilePathWithExtension); | |
| return Path.GetFileName(FullFilePathWithExtension ?? string.Empty); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| using MassSpectrometry; | ||
| using NUnit.Framework; | ||
| using Omics; | ||
| using System.Collections.Generic; | ||
| using System.Diagnostics.CodeAnalysis; | ||
|
|
||
| namespace Test.Omics | ||
| { | ||
| [TestFixture] | ||
| [ExcludeFromCodeCoverage] | ||
| internal class IsobaricQuantFileAndSampleTests | ||
| { | ||
| [Test] | ||
| public void TestIsobaricQuantFileAndPlexAnnotationCreation() | ||
| { | ||
| // Create annotations for the plex | ||
| var annotations = new List<IsobaricQuantPlexAnnotation> | ||
| { | ||
| new IsobaricQuantPlexAnnotation | ||
| { | ||
| Tag = "126", | ||
| SampleName = "Sample1", | ||
| Condition = "Control", | ||
| BiologicalReplicate = 1 | ||
| } | ||
| }; | ||
|
|
||
| // Create an instance of IsobaricQuantFileInfo | ||
| var fileInfo = new IsobaricQuantFileInfo( | ||
| fullFilePathWithExtension: "SampleQuantFile.raw", | ||
| plex: "TMT10", | ||
| fraction: 1, | ||
| technicalReplicate: 1, | ||
| annotations: annotations); | ||
|
|
||
| // Assert that the properties are set correctly | ||
| Assert.That(fileInfo.FullFilePathWithExtension, Is.EqualTo("SampleQuantFile.raw")); | ||
| Assert.That(fileInfo.Plex, Is.EqualTo("TMT10")); | ||
| Assert.That(fileInfo.Fraction, Is.EqualTo(1)); | ||
| Assert.That(fileInfo.TechnicalReplicate, Is.EqualTo(1)); | ||
| Assert.That(fileInfo.Annotations, Is.Not.Null); | ||
| Assert.That(fileInfo.Annotations.Count, Is.EqualTo(1)); | ||
| Assert.That(fileInfo.Annotations[0].Tag, Is.EqualTo("126")); | ||
| Assert.That(fileInfo.Annotations[0].SampleName, Is.EqualTo("Sample1")); | ||
| Assert.That(fileInfo.Annotations[0].Condition, Is.EqualTo("Control")); | ||
| Assert.That(fileInfo.Annotations[0].BiologicalReplicate, Is.EqualTo(1)); | ||
| } | ||
|
Comment on lines
+18
to
+51
|
||
| } | ||
| } | ||
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 constructor doesn't validate or normalize the
fullFilePathWithExtensionparameter before assigning it. UnlikeSpectraFileInfowhich normalizes null tostring.Empty, this implementation allows null to be stored directly, which could causeNullReferenceExceptionwhen callingGetHashCode()orToString()methods.Consider adding null-coalescing: