Skip to content

Commit

Permalink
Feature/cleanup (#163)
Browse files Browse the repository at this point in the history
* Start nullable checking, static YAML
* Warnings fixup, make constant data static readonly
  • Loading branch information
jas88 authored Jun 23, 2023
1 parent caf0d70 commit 7353353
Show file tree
Hide file tree
Showing 17 changed files with 219 additions and 257 deletions.
5 changes: 0 additions & 5 deletions .lgtm.yml

This file was deleted.

3 changes: 3 additions & 0 deletions BadDicom/BadDicom.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
<DebugType>embedded</DebugType>
<SymbolPackageFormat>snupkg</SymbolPackageFormat>
<PublishSingleFile>true</PublishSingleFile>
<Nullable>enable</Nullable>
<EmitCompilerGeneratedFiles>true</EmitCompilerGeneratedFiles>
</PropertyGroup>

<ItemGroup>
Expand All @@ -34,6 +36,7 @@
<ItemGroup>
<PackageReference Include="CommandLineParser" Version="2.9.1" />
<PackageReference Include="HIC.DicomTypeTranslation" Version="4.0.3" />
<PackageReference Include="Vecc.YamlDotNet.Analyzers.StaticGenerator" Version="13.0.2" />
<PackageReference Include="YamlDotNet" Version="13.1.1" />
</ItemGroup>
<ItemGroup>
Expand Down
9 changes: 6 additions & 3 deletions BadDicom/Configuration/Config.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
namespace BadDicom.Configuration;
using YamlDotNet.Serialization;

namespace BadDicom.Configuration;

[YamlSerializable]
internal class Config
{
public TargetDatabase Database { get;set; }
public ExplicitUIDs UIDs { get; set; }
public TargetDatabase? Database { get;set; }
public ExplicitUIDs? UIDs { get; set; }
}
8 changes: 8 additions & 0 deletions BadDicom/Configuration/ConfigContext.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
using YamlDotNet.Serialization;

namespace BadDicom.Configuration;

[YamlStaticContext]
public partial class ConfigContext : StaticContext
{
}
12 changes: 7 additions & 5 deletions BadDicom/Configuration/ExplicitUIDs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,28 +2,30 @@
using System.Collections.Generic;
using System.IO;
using System.Linq;
using YamlDotNet.Serialization;

namespace BadDicom.Configuration;

/// <summary>
/// Config section for loading explicit UIDs from disk and using those in file creation
/// </summary>
[YamlSerializable]
public class ExplicitUIDs
{
/// <summary>
/// Path to a file containing a list of study instance UIDs to use
/// </summary>
public string StudyInstanceUIDs { get; set; }
public string? StudyInstanceUIDs { get; set; }

/// <summary>
/// Path to a file containing a list of series instance UIDs to use
/// </summary>
public string SeriesInstanceUIDs { get; set; }
public string? SeriesInstanceUIDs { get; set; }

/// <summary>
/// Path to a file containing a list of SOP instance UIDs to use
/// </summary>
public string SOPInstanceUIDs { get; set; }
public string? SOPInstanceUIDs { get; set; }

/// <summary>
/// Loads the UID files referenced (if they exist) in the configuration
Expand All @@ -46,10 +48,10 @@ public void Load()
UIDAllocator.SOPUIDs.Enqueue(u);
}

private IEnumerable<string> GetUIDsFrom(string path)
private static IEnumerable<string> GetUIDsFrom(string? path)
{
if (string.IsNullOrWhiteSpace(path) || !File.Exists(path)) return Enumerable.Empty<string>();

return File.ReadLines(StudyInstanceUIDs).Where(l => !string.IsNullOrWhiteSpace(l));
return File.ReadLines(path).Where(l => !string.IsNullOrWhiteSpace(l));
}
}
8 changes: 5 additions & 3 deletions BadDicom/Configuration/TargetDatabase.cs
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
using FAnsi;
using YamlDotNet.Serialization;

namespace BadDicom.Configuration;

/// <summary>
/// Identify the target database and configuration for generated data
/// </summary>
[YamlSerializable]
public class TargetDatabase
{
/// <summary>
Expand All @@ -14,17 +16,17 @@ public class TargetDatabase
/// <summary>
/// The ConnectionString containing the server name, credentials and other parameters for the connection
/// </summary>
public string ConnectionString { get; set; }
public string? ConnectionString { get; set; }

/// <summary>
/// The name of database
/// </summary>
public string DatabaseName { get; set; }
public string? DatabaseName { get; set; }

/// <summary>
/// The filename of a YAML template file to be used for this database
/// </summary>
public string Template { get; set; }
public string? Template { get; set; }

/// <summary>
/// Pass true to create tables from template that do not have primary key. Do bulk insert
Expand Down
53 changes: 15 additions & 38 deletions BadDicom/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
using System.Runtime.InteropServices;
using System.Threading.Tasks;
using BadDicom.Configuration;
using FellowOakDicom;
using DicomTypeTranslation;
using DicomTypeTranslation.TableCreation;
using FAnsi.Discovery;
Expand Down Expand Up @@ -61,13 +60,12 @@ private static void RunOptionsAndReturnExitCode(ProgramOptions opts)

try
{
var d = new Deserializer();
var d = new StaticDeserializerBuilder(new ConfigContext()).Build();
config = d.Deserialize<Config>(File.ReadAllText(ConfigFile));
}
catch (Exception e)
{
Console.WriteLine($"Error deserializing '{ConfigFile}'");
Console.Write(e.ToString());
Console.WriteLine($"Error deserializing '{ConfigFile}'{Environment.NewLine}{e}");
_returnCode = -1;
return;
}
Expand Down Expand Up @@ -112,12 +110,12 @@ private static void RunOptionsAndReturnExitCode(ProgramOptions opts)
_returnCode = 0;
}

private static DicomDataGenerator GetDataGenerator(ProgramOptions opts,Random r, out DirectoryInfo dir)
private static DicomDataGenerator GetDataGenerator(ProgramOptions opts,Random r, out DirectoryInfo? dir)
{
//Generate the dicom files (of the modalities that the user requested)
var modalities = !string.IsNullOrWhiteSpace(opts.Modalities)? opts.Modalities.Split(",") :Array.Empty<string>();
var modalities = string.IsNullOrWhiteSpace(opts.Modalities)? Array.Empty<string>() :opts.Modalities.Split(",");

dir = opts.OutputDirectory.Equals("/dev/null",StringComparison.InvariantCulture) ? null : Directory.CreateDirectory(opts.OutputDirectory);
dir = opts.OutputDirectory?.Equals("/dev/null",StringComparison.InvariantCulture)!=false ? null : Directory.CreateDirectory(opts.OutputDirectory);
return new DicomDataGenerator(r, opts.OutputDirectory, modalities)
{
NoPixels = opts.NoPixels,
Expand All @@ -144,14 +142,10 @@ private static int RunDatabaseTarget(TargetDatabase configDatabase, ProgramOptio
var batchSize = Math.Max(1, configDatabase.Batches);

//if we are going into a database we definitely do not need pixels!
if (opts.NoPixels == false)
opts.NoPixels = true;
opts.NoPixels = true;


Stopwatch swTotal = new();

swTotal.Start();

var swTotal = Stopwatch.StartNew();
const string neverDistinct = "SOPInstanceUID";

if (!File.Exists(configDatabase.Template))
Expand All @@ -167,8 +161,7 @@ private static int RunDatabaseTarget(TargetDatabase configDatabase, ProgramOptio
}
catch (Exception e)
{
Console.WriteLine($"Error reading yaml from '{configDatabase.Template}'");
Console.WriteLine(e.ToString());
Console.WriteLine($"Error reading yaml from '{configDatabase.Template}'{Environment.NewLine}{e}");
return -2;
}

Expand Down Expand Up @@ -220,7 +213,7 @@ private static int RunDatabaseTarget(TargetDatabase configDatabase, ProgramOptio
for (var i = 0; i < uploaders.Length; i++)
uploaders[i] = new IBulkCopy[template.Tables.Count];

var pks = new string[template.Tables.Count];
var pks = new string?[template.Tables.Count];

for (var i = 0; i < template.Tables.Count; i++)
{
Expand Down Expand Up @@ -286,22 +279,9 @@ private static int RunDatabaseTarget(TargetDatabase configDatabase, ProgramOptio
uploaders[j][i] = tbl.BeginBulkInsert();
}
}
var tasks = new Task[batchSize];

var identifiers = GetPeople(opts, out var r);

for (var i = 0; i < batchSize; i++)
{
var batch = i;
tasks[i] = new Task(() => // lgtm[cs/local-not-disposed]
{
RunBatch(identifiers,opts,r,batches[batch],uploaders[batch]);

});
tasks[i].Start();
}

Task.WaitAll(tasks);
Parallel.For(0, batchSize, i => RunBatch(identifiers, opts, r, batches[i], uploaders[i]));

swTotal.Stop();

Expand Down Expand Up @@ -341,7 +321,7 @@ private static void RunBatch(IPersonCollection identifiers, ProgramOptions opts,
swGeneration.Start();

var p = identifiers.People[r.Next(identifiers.People.Length)];
var ds = dicomGenerator.GenerateStudyImages(p,out var s);
var ds = dicomGenerator.GenerateStudyImages(p,out _);

swGeneration.Stop();

Expand All @@ -358,11 +338,8 @@ private static void RunBatch(IPersonCollection identifiers, ProgramOptions opts,
var column = DicomTypeTranslaterReader.GetColumnNameForTag(item.Tag, false);
var value = DicomTypeTranslater.Flatten(DicomTypeTranslaterReader.GetCSharpValue(dataset, item));

foreach (var row in rows)
{
if (row.Table.Columns.Contains(column))
row[column] = value ?? DBNull.Value;
}
foreach (var row in rows.Where(row=>row.Table.Columns.Contains(column)))
row[column] = value ?? DBNull.Value;
}

for (var j = 0; j < batches.Length; j++)
Expand All @@ -372,7 +349,7 @@ private static void RunBatch(IPersonCollection identifiers, ProgramOptions opts,
}

//every 100 and last batch
if (i % 100 == 0 || i == opts.NumberOfStudies - 1)
if (i % 100 != 0 && i != opts.NumberOfStudies - 1) continue;
{
swUploading.Start();
for (var j = 0; j < uploaders.Length; j++)
Expand All @@ -383,7 +360,7 @@ private static void RunBatch(IPersonCollection identifiers, ProgramOptions opts,
swUploading.Stop();
Console.WriteLine($"{DateTime.Now} Done {i} studies");
}

}
}
finally
Expand Down
2 changes: 1 addition & 1 deletion BadDicom/ProgramOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ namespace BadDicom;
internal class ProgramOptions
{
[Value(0,HelpText = "Output directory to create CSV files in",Required=true)]
public string OutputDirectory { get; set; }
public string? OutputDirectory { get; set; }

[Value(1, HelpText = "The number of unique patient identifiers to generate up front and then use in test data",Default = 500)]
public int NumberOfPatients { get; set; } = 500;
Expand Down
5 changes: 1 addition & 4 deletions BadMedicine.Dicom.Tests/DicomDataGeneratorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public void Test_Anonymise()
var r = new Random(23);
var person = new Person(r);

var generator = new DicomDataGenerator(r,new string(TestContext.CurrentContext.WorkDirectory),"CT");
using var generator = new DicomDataGenerator(r,new string(TestContext.CurrentContext.WorkDirectory),"CT");

// without anonymisation (default) we get the normal patient ID
var ds = generator.GenerateTestDataset(person, r);
Expand All @@ -100,9 +100,6 @@ public void Test_Anonymise()
// we get a blank patient ID
Assert.IsTrue(ds2.Contains(DicomTag.PatientID));
Assert.AreEqual(string.Empty,ds2.GetString(DicomTag.PatientID));

generator.Dispose();

}
[Test]
public void Test_CreatingInMemory_Modality_CTAndMR()
Expand Down
1 change: 1 addition & 0 deletions BadMedicine.Dicom/BadMedicine.Dicom.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
<PackageLicenseExpression>GPL-3.0-or-later</PackageLicenseExpression>
<Copyright>Copyright 2019</Copyright>
<PackageTags>DICOM,Test Data,Random,Synthetic Data,Health</PackageTags>
<Nullable>enable</Nullable>
</PropertyGroup>

<ItemGroup>
Expand Down
6 changes: 3 additions & 3 deletions BadMedicine.Dicom/DescBodyPart.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,17 @@ public class DescBodyPart
/// A known value of <see cref="DicomTag.StudyDescription"/> which is consistent with
/// <see cref="BodyPartExamined"/> and <see cref="SeriesDescription"/> (of this class)
/// </summary>
public string StudyDescription { get; init; }
public string? StudyDescription { get; init; }

/// <summary>
/// A known value of <see cref="DicomTag.BodyPartExamined"/> which is consistent with
/// <see cref="StudyDescription"/> and <see cref="SeriesDescription"/> (of this class)
/// </summary>
public string BodyPartExamined { get; init; }
public string? BodyPartExamined { get; init; }

/// <summary>
/// A known value of <see cref="DicomTag.SeriesDescription"/> which is consistent with
/// <see cref="BodyPartExamined"/> and <see cref="StudyDescription"/> (of this class)
/// </summary>
public string SeriesDescription { get; init; }
public string? SeriesDescription { get; init; }
}
Loading

0 comments on commit 7353353

Please sign in to comment.