Make modality required for all extractions

bin/common.py

@@ -4,6 +4,7 @@
 import functools
 import hashlib
 import os
+import shlex
 import subprocess
 from collections.abc import Sequence
 
@@ -37,7 +38,7 @@ def run(
     if env is None:
         env = {}
 
-    subprocess.check_call(("echo", "$", *cmd), cwd=cwd)
+    print(shlex.join(("$", *cmd)))
     subprocess.check_call(cmd, cwd=cwd, env={**os.environ, **env})
 
 

data/microserviceConfigs/default.yaml

@@ -76,7 +76,7 @@ CohortExtractorOptions:
   #RejectColumnInfos: [105,110]
 
   AuditorType: "SmiServices.Microservices.CohortExtractor.Audit.NullAuditExtractions"
-  RequestFulfillerType: "SmiServices.Microservices.CohortExtractor.RequestFulfillers.FromCataloguesExtractionRequestFulfiller"
+  RequestFulfillerType: "FromCataloguesExtractionRequestFulfiller"
   ProjectPathResolverType: "StudySeriesSOPProjectPathResolver"
   ExtractAnonRoutingKey: anon
   ExtractIdentRoutingKey: ident

news/2045-change.md

@@ -0,0 +1 @@
+Make modality required for all extractions

src/SmiServices/Applications/ExtractImages/ExtractImagesCliOptions.cs

@@ -19,12 +19,11 @@ public class ExtractImagesCliOptions : CliOptions
             HelpText = "The CSV file containing IDs of the cohort for extraction")]
         public string CohortCsvFile { get; set; } = null!;
 
-        // Optional
+        [Option(shortName: 'm', longName: "modality", Required = true,
+            HelpText = "The modality to extract. Any non-matching IDs from the input list are ignored")]
+        public string Modality { get; set; } = null!;
 
-        [Option(shortName: 'm', longName: "modalities", Required = false,
-            HelpText =
-                "[Optional] List of modalities to extract. Any non-matching IDs from the input list are ignored")]
-        public string? Modalities { get; set; }
+        // Optional
 
         [Option(shortName: 'i', longName: "identifiable-extraction", Required = false,
             HelpText = "Extract without performing anonymisation")]
@@ -56,7 +55,7 @@ public static IEnumerable<Example> Examples
                     {
                         CohortCsvFile = "my.csv",
                         ProjectId = "1234-5678",
-                        Modalities = "CT",
+                        Modality = "CT",
                         IsIdentifiableExtraction = true
                     });
                 yield return new Example(helpText: "Extract without applying any rejection filters",
@@ -72,7 +71,7 @@ public override string ToString()
             sb.Append(base.ToString());
             sb.Append($"ProjectId={ProjectId},");
             sb.Append($"CohortCsvFile={CohortCsvFile},");
-            sb.Append($"Modalities={Modalities},");
+            sb.Append($"Modality={Modality},");
             sb.Append($"IdentifiableExtraction={IsIdentifiableExtraction},");
             sb.Append($"NoFiltersExtraction={IsNoFiltersExtraction},");
             sb.Append($"NonInteractive={NonInteractive},");

src/SmiServices/Applications/ExtractImages/ExtractionMessageSender.cs

@@ -27,7 +27,7 @@ public class ExtractionMessageSender : IExtractionMessageSender
         private readonly int _maxIdentifiersPerMessage;
 
         private readonly string _projectId;
-        private readonly string[]? _modalities;
+        private readonly string _modality;
         private readonly bool _isIdentifiableExtraction;
         private readonly bool _isNoFiltersExtraction;
         private readonly bool _isPooledExtraction;
@@ -60,7 +60,7 @@ IConsoleInput consoleInput
                 throw new ArgumentOutOfRangeException(nameof(options));
 
             _projectId = (!string.IsNullOrWhiteSpace(cliOptions.ProjectId)) ? cliOptions.ProjectId : throw new ArgumentOutOfRangeException(nameof(cliOptions));
-            _modalities = cliOptions.Modalities?.ToUpper().Split(',', StringSplitOptions.RemoveEmptyEntries);
+            _modality = (!string.IsNullOrWhiteSpace(cliOptions.Modality)) ? cliOptions.Modality : throw new ArgumentOutOfRangeException(nameof(cliOptions));
             _isIdentifiableExtraction = cliOptions.IsIdentifiableExtraction;
             _isNoFiltersExtraction = cliOptions.IsNoFiltersExtraction;
             _isPooledExtraction = cliOptions.IsPooledExtraction;
@@ -75,8 +75,6 @@ public void SendMessages(ExtractionKey extractionKey, List<string> idList)
             var jobId = Guid.NewGuid();
             DateTime now = _dateTimeProvider.UtcNow();
 
-            // TODO(rkm 2021-04-01) Change this to a string[] in both messages below
-            string? modalitiesString = _modalities == null ? null : string.Join(',', _modalities);
             string userName = Environment.UserName;
 
             var erm = new ExtractionRequestMessage
@@ -92,7 +90,7 @@ public void SendMessages(ExtractionKey extractionKey, List<string> idList)
                 // TODO(rkm 2021-04-01) Change this to an ExtractionKey type
                 KeyTag = extractionKey.ToString(),
 
-                Modalities = modalitiesString,
+                Modality = _modality,
 
                 // NOTE(rkm 2021-04-01) Set below
                 ExtractionIdentifiers = null!,
@@ -113,6 +111,7 @@ public void SendMessages(ExtractionKey extractionKey, List<string> idList)
                 ExtractionJobIdentifier = jobId,
                 ProjectNumber = _projectId,
                 ExtractionDirectory = _extractionDir,
+                Modality = _modality,
                 JobSubmittedAt = now,
                 IsIdentifiableExtraction = _isIdentifiableExtraction,
                 IsNoFilterExtraction = _isNoFiltersExtraction,
@@ -121,7 +120,6 @@ public void SendMessages(ExtractionKey extractionKey, List<string> idList)
                 KeyTag = extractionKey.ToString(),
                 KeyValueCount = idList.Count,
                 UserName = userName,
-                ExtractionModality = modalitiesString,
             };
 
             if (_nonInteractive)
@@ -136,11 +134,11 @@ public void SendMessages(ExtractionKey extractionKey, List<string> idList)
                 sb.AppendLine($"Submitted:                      {now:u}");
                 sb.AppendLine($"ProjectNumber:                  {_projectId}");
                 sb.AppendLine($"ExtractionDirectory:            {_extractionDir}");
+                sb.AppendLine($"Modality:                       {_modality}");
                 sb.AppendLine($"ExtractionKey:                  {extractionKey}");
                 sb.AppendLine($"IsIdentifiableExtraction:       {_isIdentifiableExtraction}");
                 sb.AppendLine($"IsNoFilterExtraction:           {_isNoFiltersExtraction}");
                 sb.AppendLine($"IsPooledExtraction:             {_isPooledExtraction}");
-                sb.AppendLine($"ExtractionModality:             {modalitiesString ?? "<unspecified>"}");
                 sb.AppendLine($"UserName:                       {userName}");
                 sb.AppendLine($"KeyValueCount:                  {idList.Count}");
                 sb.AppendLine($"ExtractionRequestMessage count: {ermList.Count}");

src/SmiServices/Common/Messages/Extraction/ExtractFileMessage.cs

@@ -21,12 +21,6 @@ public class ExtractFileMessage : ExtractMessage, IFileReferenceMessage
         [JsonProperty(Required = Required.Always)]
         public string OutputPath { get; set; } = null!;
 
-        /// <summary>
-        /// The modality of the specified file
-        /// </summary>
-        [JsonProperty(Required = Required.AllowNull)]
-        public string Modality { get; set; } = null!;
-
 
         [JsonConstructor]
         public ExtractFileMessage() { }

src/SmiServices/Common/Messages/Extraction/ExtractMessage.cs

@@ -18,6 +18,9 @@ public abstract class ExtractMessage : MemberwiseEquatable<ExtractMessage>, IExt
         [JsonProperty(Required = Required.Always)]
         public string ExtractionDirectory { get; set; } = null!;
 
+        [JsonProperty(Required = Required.Always)]
+        public string Modality { get; set; } = null!;
+
         [JsonProperty(Required = Required.Always)]
         public DateTime JobSubmittedAt { get; set; }
 
@@ -39,6 +42,7 @@ protected ExtractMessage(
             Guid extractionJobIdentifier,
             string projectNumber,
             string extractionDirectory,
+            string modality,
             DateTime jobSubmittedAt,
             bool isIdentifiableExtraction,
             bool isNoFilterExtraction,
@@ -49,6 +53,7 @@ bool isPooledExtraction
             ExtractionJobIdentifier = extractionJobIdentifier;
             ProjectNumber = projectNumber;
             ExtractionDirectory = extractionDirectory;
+            Modality = modality;
             JobSubmittedAt = jobSubmittedAt;
             IsIdentifiableExtraction = isIdentifiableExtraction;
             IsNoFilterExtraction = isNoFilterExtraction;
@@ -60,6 +65,7 @@ protected ExtractMessage(IExtractMessage request)
                 request.ExtractionJobIdentifier,
                 request.ProjectNumber,
                 request.ExtractionDirectory,
+                request.Modality,
                 request.JobSubmittedAt,
                 request.IsIdentifiableExtraction,
                 request.IsNoFilterExtraction,
@@ -71,6 +77,7 @@ public override string ToString() =>
             $"ExtractionJobIdentifier={ExtractionJobIdentifier}, " +
             $"ProjectNumber={ProjectNumber}, " +
             $"ExtractionDirectory={ExtractionDirectory}, " +
+            $"Modality={Modality}, " +
             $"JobSubmittedAt={JobSubmittedAt:s}, " +
             $"IsIdentifiableExtraction={IsIdentifiableExtraction}, " +
             $"IsNoFilterExtraction={IsNoFilterExtraction}, " +

src/SmiServices/Common/Messages/Extraction/ExtractionRequestInfoMessage.cs

@@ -13,15 +13,13 @@ public class ExtractionRequestInfoMessage : ExtractMessage
         [JsonProperty(Required = Required.Always)]
         public string UserName { get; set; } = null!;
 
-        [JsonProperty(Required = Required.Default)]
-        public string? ExtractionModality { get; set; }
 
         [JsonConstructor]
         public ExtractionRequestInfoMessage() { }
 
         public override string ToString()
         {
-            return base.ToString() + $",KeyTag={KeyTag},KeyValueCount={KeyValueCount},UserName={UserName},ExtractionModality={ExtractionModality}";
+            return base.ToString() + $",KeyTag={KeyTag},KeyValueCount={KeyValueCount},UserName={UserName}";
         }
     }
 }

src/SmiServices/Common/Messages/Extraction/ExtractionRequestMessage.cs

@@ -14,13 +14,6 @@ public class ExtractionRequestMessage : ExtractMessage
         [JsonProperty(Required = Required.Always)]
         public string KeyTag { get; set; } = null!;
 
-        /// <summary>
-        /// Optional list of modalities to extract when <see cref="KeyTag"/> could include multiple modalities
-        /// (e.g. extracting based on patient or study (study can include e.g. CT + SR).
-        /// </summary>
-        [JsonProperty(Required = Required.AllowNull)]
-        public string? Modalities { get; set; }
-
         /// <summary>
         /// The unique set of identifiers of Type <see cref="KeyTag"/> which should be extracted
         /// </summary>
@@ -41,14 +34,12 @@ public ExtractionRequestMessage(ExtractionRequestMessage other)
         : base(other)
         {
             KeyTag = other.KeyTag;
-            Modalities = other.Modalities;
             ExtractionIdentifiers = other.ExtractionIdentifiers;
         }
 
         public override string ToString()
             => base.ToString() + ", " +
                $"KeyTag={KeyTag}, " +
-               $"Modality={Modalities ?? "Unspecified"}, " +
                $"nIdentifiers={ExtractionIdentifiers.Count}";
     }
 }

src/SmiServices/Common/Messages/Extraction/IExtractMessage.cs

@@ -1,3 +1,4 @@
+using Newtonsoft.Json;
 using System;
 
 namespace SmiServices.Common.Messages.Extraction
@@ -22,6 +23,11 @@ public interface IExtractMessage : IMessage
         /// </summary>
         string ExtractionDirectory { get; }
 
+        /// <summary>
+        /// The modality to extract
+        /// </summary>
+        public string Modality { get; }
+
         /// <summary>
         /// DateTime the job was submitted at
         /// </summary>

src/SmiServices/Common/Options/GlobalOptions.cs

@@ -363,15 +363,13 @@ public string AuditorType
         /// The Type of a class implementing IProjectPathResolver which is responsible for deciding the folder hierarchy to output into
         /// </summary>
         public string? ProjectPathResolverType { get; set; }
-
-        public const string DefaultModalityRoutingRegex = "^([A-Z]+)_.*$";
-
+
         /// <summary>
         /// Controls how modalities are matched to Catalogues.  Must contain a single capture group which
         /// returns a modality code (e.g. CT) when applies to a Catalogue name.  E.g. ^([A-Z]+)_.*$ would result
         /// in Modalities being routed based on the start of the table name e.g. CT => CT_MyTable and MR=> MR_MyTable
         /// </summary>
-        public string? ModalityRoutingRegex { get; set; } = DefaultModalityRoutingRegex;
+        public string? ModalityRoutingRegex { get; set; }
 
         /// <summary>
         /// The Type of a class implementing IRejector which is responsible for deciding individual records/images are not extractable (after fetching from database)

src/SmiServices/Microservices/CohortExtractor/CohortExtractor.cs

@@ -20,7 +20,6 @@ private static int OnParse(GlobalOptions globals, CliOptions opts)
             var bootstrapper = new MicroserviceHostBootstrapper(
                 () => new CohortExtractorHost(
                     globals,
-                    auditor: null,
                     fulfiller: null
                 )
             );