Console method files: load the RT correction library once from the final path; make duplicated keys last-wins everywhere - #817
Merged
Conversation
…ery method-file key last-wins The console method-file reader loaded "Compounds library file path for RT correction" the moment it read the key, from the value as written: a relative value was opened from the working directory, and GC-MS path resolution (or the rt-correction command's own library argument) could leave the stored library and the stored path naming different files. The reader now records only the path; RetentionTimeCorrectionProcess.LoadStandards is the single load, after every rewrite. The six settings LcmsProcess reads for itself took the FIRST usable line while every other key takes the last. They now take the last usable line, skipping blank and unusable values the way the main readers do. The key record gains a "repeated" list naming each key written on more than one line and the value the run used, and the console says the same. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The parse-time load also served as an early format check: the library is small and easy to get wrong, so it should fail before the raw data and annotation libraries are read. LcmsProcess now loads it with LoadStandards right after the method file is read, before the analysis files are imported, and hands the anchors to Prepare, which no longer reads the file again. The rt-correction command uses the same loader for its library argument. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With Execute RT correction: False the library has no effect, so LcmsProcess neither reads nor checks it, but a path left in the method file usually means the switch was meant to be on, so the run says so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two fixes to the MS-DIAL 5 console's method-file reader (
tests/MSDIAL5/MsdialCoreTestApp/Parser/ConfigParser.cs).1. The RT correction library is no longer loaded while the file is being read
ReadCommonParameterused to callTextLibraryParser.StandardTextLibraryReaderas soon as it readCompounds library file path for RT correction, using the value exactly as written, and stored the result inRetentionTimeCorrectionCommon.StandardLibrary. That caused three problems:ResolveGcmsFilePaths), and thert-correctioncommand swaps in its own library argument after parsing. In both cases the stored library and the stored path could refer to different files;null, soStandardLibrarybecamenull.Who reads
StandardLibrary: in the console and the core code it calls, onlyRetentionTimeCorrectionProcess.Prepare. It setsStandardLibrary, andMsdialCore'sRetentionTimeCorrection.Executethen reads it.Executeis reached only fromPrepare.ParameterBase.ParametersAsTextreads it too, but the console never calls that.Preparealready reloaded the library fromCompoundListForRtCorrectionPathwheneverExecute RT correction: True.Change: the parser now records only the path.
RetentionTimeCorrectionProcess.LoadStandardsis the one place the library is loaded, and it is called from the final path after every rewrite, so it stays compatible with a separate change that rewrites relative paths after reading. The load is still an early format check: the library is small and its format is easy to get wrong, soLcmsProcessloads it right after the method file is read, before the analysis files are imported and before the annotation libraries and raw data. A malformed library stops the run there withRT correction library could not be used: <parser message>. The loaded anchors are passed toPrepare(newstandardLibraryargument), which no longer reads the file again. Thert-correctioncommand uses the same loader for its library argument, before it reads any raw data, and passes its anchors toPreparetoo, so that command also reads the file once.LoadStandardsisinternalso tests can call it.2. Duplicated keys now resolve the same way for every key
ReadCommonParameterand the mode readers keep the last line of a key. The six separate LC-MS passes kept the first usable line:ReadAlignmentLightMode,ReadLbmAnnotatorPriority,ReadDetailedAlignmentProvenance,ReadAnnotationCandidateExport, and the MSP and Text annotator settings-file-path readers.ReadFirstis replaced byReadLast. It keeps the last line the line reader accepts and skips blank and unusable values, which is how the main readers already behave.Key report:
MethodFileKeysnow lists each key written on more than one line (compared ignoring letter case) and the value the run used, meaning the last line a reader accepted. This goes in a newrepeatedarray in<method>.keys.json({"key", "lines", "used"}, withused: nullwhen no line applied) and in a console line:Method file 'x.txt': the parameter 'Minimum peak height' is written on 2 lines; the last line applied was used: '200'.The addition is additive, so the schema stays
msdial-method-file-keys.v1.Left out of scope:
LBM annotator priority/LBM annotation priority) are not matched as the same key, because only the readers know which spellings belong together.Ion mode,MS1 data type) still return "applied" for a value they ignore. For those,usedcan name a value that was never assigned. That is an existing inaccuracy of those arms and is not new here.Behaviour changes for existing method files
StandardLibrarystays empty and a project saved by the console no longer carries standards for a correction that was not run. If the path is set anyway, LC-MS printsWarning: 'Compounds library file path for RT correction' is set (…) but 'Execute RT correction' is False, so the library is not used.in place of the old parser error print.RT correction library could not be used: …before the analysis files are loaded, instead of a parse-time print followed byRT correction failedafter the import.StandardLibraryat all. None of them runs RT correction in the console, so only a saved project's contents change.MSP/Text annotator settings file path:) no longer ends the search. A real path on a later line now applies, and a blank line after a real path no longer hides it. The old code stopped at the blank line, so before this change a blank line followed by a real path gave no settings file.repeatedentries and lines described above.Tests
New tests in
ConfigParserTests:the side readers take the last line of a key written twice, for all six settings, and agree with the main reader;
a blank or unusable later line leaves the earlier value, and a blank first line no longer blocks a later path;
the key record and console report name repeated keys and the value used, including cases where no line applied;
LC-MS parsing records the RT library path without loading it;
for GC-MS, a relative RT library path is resolved against the method file's folder and
LoadStandardsloads it from there;a malformed library is refused with the parser's message;
an LC-MS run with a malformed library stops before
Loading analysis files..;with RT correction off, a set library path is warned about and not read, and nothing is said when no path is set.
dotnet test tests/MSDIAL5/MsdialCoreTestAppTests/MsdialCoreTestAppTests.csproj: 77 passed, 0 faileddotnet build tests/MSDIAL5/MsdialCoreTestApp/MsdialCoreTestApp.csproj(net472, net48, net8): succeeded, 0 errors🤖 Generated with Claude Code