From 07d8014772a732ff86870f0d2f49f1b0fe3a9781 Mon Sep 17 00:00:00 2001 From: Melissa Linkert Date: Tue, 30 May 2023 15:46:20 -0500 Subject: [PATCH 01/18] Initial version of interface and writer API for accepting extra DICOM tags --- .../src/loci/formats/dicom/DicomTag.java | 17 +++++++ .../src/loci/formats/dicom/ITagProvider.java | 47 +++++++++++++++++++ .../src/loci/formats/out/DicomWriter.java | 37 +++++++++++++++ 3 files changed, 101 insertions(+) create mode 100644 components/formats-bsd/src/loci/formats/dicom/ITagProvider.java diff --git a/components/formats-bsd/src/loci/formats/dicom/DicomTag.java b/components/formats-bsd/src/loci/formats/dicom/DicomTag.java index b96fa9986d7..3714cdedb10 100644 --- a/components/formats-bsd/src/loci/formats/dicom/DicomTag.java +++ b/components/formats-bsd/src/loci/formats/dicom/DicomTag.java @@ -522,6 +522,23 @@ public DicomTag lookupChild(DicomAttribute attr) { return null; } + /** + * Check this tag against a list of existing tags. + * If this tag is a duplicate of an existing tag or otherwise deemed invalid, + * return false. Otherwise return true. + */ + public boolean validate(List tags) { + // TODO: this is very simplistic validation that just rejects duplicates + // we could pursue more comprehensive validation here, + // but need to define what that means + for (DicomTag t : tags) { + if (this.tag == t.tag) { + return false; + } + } + return true; + } + @Override public String toString() { return key + " = " + value; diff --git a/components/formats-bsd/src/loci/formats/dicom/ITagProvider.java b/components/formats-bsd/src/loci/formats/dicom/ITagProvider.java new file mode 100644 index 00000000000..0dbdf6c72ce --- /dev/null +++ b/components/formats-bsd/src/loci/formats/dicom/ITagProvider.java @@ -0,0 +1,47 @@ +/* + * #%L + * BSD implementations of Bio-Formats readers and writers + * %% + * Copyright (C) 2005 - 2023 Open Microscopy Environment: + * - Board of Regents of the University of Wisconsin-Madison + * - Glencoe Software, Inc. + * - University of Dundee + * %% + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + * #L% + */ + +package loci.formats.dicom; + +import java.util.List; + +/** + * Interface used to provide DICOM tags. + * Implementations may parse tags from a file or other source; + * loci.formats.out.DicomWriter makes use of this interface + * to accept supplementary tags. + */ +public interface ITagProvider { + + List getTags(); + +} diff --git a/components/formats-bsd/src/loci/formats/out/DicomWriter.java b/components/formats-bsd/src/loci/formats/out/DicomWriter.java index bb20ce45106..4afb6e57923 100644 --- a/components/formats-bsd/src/loci/formats/out/DicomWriter.java +++ b/components/formats-bsd/src/loci/formats/out/DicomWriter.java @@ -57,6 +57,7 @@ import loci.formats.codec.JPEG2000Codec; import loci.formats.codec.JPEG2000CodecOptions; import loci.formats.codec.JPEGCodec; +import loci.formats.dicom.ITagProvider; import loci.formats.dicom.DicomTag; import loci.formats.in.DynamicMetadataOptions; import loci.formats.in.MetadataOptions; @@ -104,6 +105,8 @@ public class DicomWriter extends FormatWriter { private String instanceUIDValue; private String implementationUID; + private ArrayList tagProviders = new ArrayList(); + // -- Constructor -- public DicomWriter() { @@ -115,6 +118,28 @@ public DicomWriter() { }; } + // -- DicomWriter API methods -- + + /** + * Provide additional DICOM tags that should be written + * as part of this dataset. + * + * Calling this method is optional. Multiple calls are allowed, + * e.g. if tags come from multiple external sources. + * If multiple calls to this method are made then the order matters, + * with earlier calls implying higher priority when resolving duplicate + * or conflicting tags. + * + * All calls to this method must occur before setId is called. + */ + public void setExtraTags(ITagProvider tagProvider) { + FormatTools.assertId(currentId, false, 1); + + if (tagProvider != null) { + tagProviders.add(tagProvider); + } + } + // -- IFormatWriter API methods -- @Override @@ -896,6 +921,16 @@ public void setId(String id) throws FormatException, IOException { tags.add(labelText); } + // now add all supplementary tags from tag providers + for (ITagProvider provider : tagProviders) { + for (DicomTag t : provider.getTags()) { + boolean validTag = t.validate(tags); + if (validTag) { + tags.add(t); + } + } + } + // sort tags into ascending order, then write tags.sort(new Comparator() { @@ -929,6 +964,8 @@ public void close() throws IOException { compressionMethodPointer = null; fileMetaLengthPointer = 0; + tagProviders.clear(); + // intentionally don't reset tile dimensions } From 280b43dafd1e8a602982e6e141712978e8f8e7a2 Mon Sep 17 00:00:00 2001 From: Melissa Linkert Date: Tue, 30 May 2023 17:06:58 -0500 Subject: [PATCH 02/18] Add initial implementation of ITagProvider that parses dcdump Not fully functional, but a place to start connecting bfconvert. --- .../loci/formats/dicom/DCDumpProvider.java | 92 +++++++++++++++++++ .../src/loci/formats/dicom/ITagProvider.java | 11 +++ .../src/loci/formats/out/DicomWriter.java | 3 + 3 files changed, 106 insertions(+) create mode 100644 components/formats-bsd/src/loci/formats/dicom/DCDumpProvider.java diff --git a/components/formats-bsd/src/loci/formats/dicom/DCDumpProvider.java b/components/formats-bsd/src/loci/formats/dicom/DCDumpProvider.java new file mode 100644 index 00000000000..fe2db38b5eb --- /dev/null +++ b/components/formats-bsd/src/loci/formats/dicom/DCDumpProvider.java @@ -0,0 +1,92 @@ +/* + * #%L + * BSD implementations of Bio-Formats readers and writers + * %% + * Copyright (C) 2005 - 2023 Open Microscopy Environment: + * - Board of Regents of the University of Wisconsin-Madison + * - Glencoe Software, Inc. + * - University of Dundee + * %% + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + * #L% + */ + +package loci.formats.dicom; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + +import loci.common.Constants; +import loci.common.DataTools; + +/** + * Provide DICOM tags from a file containing the output of dcdump. + */ +public class DCDumpProvider implements ITagProvider { + + private List tags = new ArrayList(); + + @Override + public void readTagSource(String location) throws IOException { + String[] lines = DataTools.readFile(location).split("\r\n"); + + // TODO: does not currently handle tag hierarchies + for (String line : lines) { + String[] tokens = line.split(" "); + String[] tag = tokens[0].replaceAll("()", "").split(","); + + String vr = tokens[1]; + vr = vr.substring(vr.indexOf("<") + 1, vr.indexOf(">")); + + String length = null; + String value = ""; + for (int i=2; i")) { + i++; + value += tokens[i]; + } + } + } + + int tagUpper = Integer.parseInt(tag[0], 16); + int tagLower = Integer.parseInt(tag[1], 16); + int vrCode = DataTools.bytesToShort(vr.getBytes(Constants.ENCODING), false); + + DicomTag t = new DicomTag(DicomAttribute.get(tagUpper << 16 | tagLower), DicomVR.get(vrCode)); + // TODO: fix value handling for more complex cases (e.g. arrays) + t.value = value; + tags.add(t); + } + } + + @Override + public List getTags() { + return tags; + } + +} diff --git a/components/formats-bsd/src/loci/formats/dicom/ITagProvider.java b/components/formats-bsd/src/loci/formats/dicom/ITagProvider.java index 0dbdf6c72ce..09eb71a393d 100644 --- a/components/formats-bsd/src/loci/formats/dicom/ITagProvider.java +++ b/components/formats-bsd/src/loci/formats/dicom/ITagProvider.java @@ -32,6 +32,7 @@ package loci.formats.dicom; +import java.io.IOException; import java.util.List; /** @@ -42,6 +43,16 @@ */ public interface ITagProvider { + /** + * Read tags from the given location. + * The interpretation of 'location' is implementation-dependent. + * This method must be called before retrieving tags. + */ + void readTagSource(String location) throws IOException; + + /** + * Retrieve the tags defined by this provider. + */ List getTags(); } diff --git a/components/formats-bsd/src/loci/formats/out/DicomWriter.java b/components/formats-bsd/src/loci/formats/out/DicomWriter.java index 4afb6e57923..1f88ca58f9a 100644 --- a/components/formats-bsd/src/loci/formats/out/DicomWriter.java +++ b/components/formats-bsd/src/loci/formats/out/DicomWriter.java @@ -928,6 +928,9 @@ public void setId(String id) throws FormatException, IOException { if (validTag) { tags.add(t); } + else { + LOGGER.warn("Ignoring tag {} from provider {}", t, provider); + } } } From 2c2998e30f0e6e61f8a620e2f70c3c88e2483683 Mon Sep 17 00:00:00 2001 From: Melissa Linkert Date: Tue, 30 May 2023 20:11:49 -0500 Subject: [PATCH 03/18] Add -extra-metadata option for bfconvert Fiddled with the API a bit to make this work in a more extensible way. Supplying an extra metadata location is no longer tied to DicomWriter specifically, but is in an extra interface that can be implemented by writers that support this feature. Considered implementing bfconvert connectivity via an option in DicomWriter, instead of a new command line argument in bfconvert. Either way would work, but the approach here would allow us to implement a similar feature in other writers later on (if we choose to do so). --- .../loci/formats/tools/ImageConverter.java | 15 ++++++ .../src/loci/formats/out/DicomWriter.java | 39 ++++++++------ .../formats/out/IExtraMetadataWriter.java | 53 +++++++++++++++++++ 3 files changed, 90 insertions(+), 17 deletions(-) create mode 100644 components/formats-bsd/src/loci/formats/out/IExtraMetadataWriter.java diff --git a/components/bio-formats-tools/src/loci/formats/tools/ImageConverter.java b/components/bio-formats-tools/src/loci/formats/tools/ImageConverter.java index f6e186b9093..c4212f03917 100644 --- a/components/bio-formats-tools/src/loci/formats/tools/ImageConverter.java +++ b/components/bio-formats-tools/src/loci/formats/tools/ImageConverter.java @@ -78,6 +78,7 @@ import loci.formats.meta.IPyramidStore; import loci.formats.out.DicomWriter; import loci.formats.ome.OMEPyramidStore; +import loci.formats.out.IExtraMetadataWriter; import loci.formats.out.TiffWriter; import loci.formats.services.OMEXMLService; import loci.formats.services.OMEXMLServiceImpl; @@ -130,6 +131,8 @@ public final class ImageConverter { private String swapOrder = null; private Byte fillColor = null; + private String extraMetadata = null; + private IFormatReader reader; private MinMaxCalculator minMax; private DimensionSwapper dimSwapper; @@ -264,6 +267,9 @@ else if (args[i].equals("-fill")) { // allow specifying 0-255 fillColor = (byte) Integer.parseInt(args[++i]); } + else if (args[i].equals("-extra-metadata")) { + extraMetadata = args[++i]; + } else if (!args[i].equals(CommandLineTools.NO_UPGRADE_CHECK)) { LOGGER.error("Found unknown command flag: {}; exiting.", args[i]); return false; @@ -638,6 +644,15 @@ else if (writer instanceof ImageWriter) { ((TiffWriter) w).setCanDetectBigTiff(!nobigtiff); } } + if (writer instanceof IExtraMetadataWriter) { + ((IExtraMetadataWriter) writer).setExtraMetadata(extraMetadata); + } + else if (writer instanceof ImageWriter) { + IFormatWriter w = ((ImageWriter) writer).getWriter(out); + if (w instanceof IExtraMetadataWriter) { + ((IExtraMetadataWriter) w).setExtraMetadata(extraMetadata); + } + } String format = writer.getFormat(); LOGGER.info("[{}] -> {} [{}]", diff --git a/components/formats-bsd/src/loci/formats/out/DicomWriter.java b/components/formats-bsd/src/loci/formats/out/DicomWriter.java index 1f88ca58f9a..182ecf1a41e 100644 --- a/components/formats-bsd/src/loci/formats/out/DicomWriter.java +++ b/components/formats-bsd/src/loci/formats/out/DicomWriter.java @@ -57,6 +57,7 @@ import loci.formats.codec.JPEG2000Codec; import loci.formats.codec.JPEG2000CodecOptions; import loci.formats.codec.JPEGCodec; +import loci.formats.dicom.DCDumpProvider; import loci.formats.dicom.ITagProvider; import loci.formats.dicom.DicomTag; import loci.formats.in.DynamicMetadataOptions; @@ -76,7 +77,7 @@ * This is designed for whole slide images, and may not produce * schema-compliant files for other modalities. */ -public class DicomWriter extends FormatWriter { +public class DicomWriter extends FormatWriter implements IExtraMetadataWriter { // -- Constants -- @@ -118,25 +119,29 @@ public DicomWriter() { }; } - // -- DicomWriter API methods -- + // -- IExtraMetadataWriter API methods -- - /** - * Provide additional DICOM tags that should be written - * as part of this dataset. - * - * Calling this method is optional. Multiple calls are allowed, - * e.g. if tags come from multiple external sources. - * If multiple calls to this method are made then the order matters, - * with earlier calls implying higher priority when resolving duplicate - * or conflicting tags. - * - * All calls to this method must occur before setId is called. - */ - public void setExtraTags(ITagProvider tagProvider) { + @Override + public void setExtraMetadata(String tagSource) { FormatTools.assertId(currentId, false, 1); - if (tagProvider != null) { - tagProviders.add(tagProvider); + // get the provider (parser) from the source name + // uses the file extension, this might need improvement + + if (tagSource != null) { + ITagProvider provider = null; + if (checkSuffix(tagSource, "dcdump")) { + provider = new DCDumpProvider(); + } + // TODO: allow for JSON parser here + + try { + provider.readTagSource(tagSource); + tagProviders.add(provider); + } + catch (IOException e) { + LOGGER.error("Could not parse extra metadata: " + tagSource, e); + } } } diff --git a/components/formats-bsd/src/loci/formats/out/IExtraMetadataWriter.java b/components/formats-bsd/src/loci/formats/out/IExtraMetadataWriter.java new file mode 100644 index 00000000000..7cc1da2170d --- /dev/null +++ b/components/formats-bsd/src/loci/formats/out/IExtraMetadataWriter.java @@ -0,0 +1,53 @@ +/* + * #%L + * BSD implementations of Bio-Formats readers and writers + * %% + * Copyright (C) 2023 Open Microscopy Environment: + * - Board of Regents of the University of Wisconsin-Madison + * - Glencoe Software, Inc. + * - University of Dundee + * %% + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + * #L% + */ + +package loci.formats.out; + +import java.io.IOException; + +public interface IExtraMetadataWriter { + + /** + * Provide additional metadata that should be written + * as part of this dataset. Primarily intended for DicomWriter. + * + * Calling this method is optional. Multiple calls are allowed, + * e.g. if tags come from multiple external sources. + * If multiple calls to this method are made then the order matters, + * with earlier calls implying higher priority when resolving duplicate + * or conflicting tags. + * + * All calls to this method must occur before setId is called. + */ + void setExtraMetadata(String metadataSource); + +} From 85433d32172110c8944c564721b7caf4bf4dfd18 Mon Sep 17 00:00:00 2001 From: Melissa Linkert Date: Wed, 31 May 2023 18:46:44 -0500 Subject: [PATCH 04/18] Add basic JSON provider for DICOM tags ...and fix up some minor issues. A simple test like this should now work: $ cat test.json { "BodyPartExamined": { "Value": "BRAIN", "VR": "CS", "Tag": "(0018,0015)" } } $ bfconvert -extra-metadata test.json test.fake test.dcm $ dcdump test.dcm --- components/formats-bsd/pom.xml | 7 ++ .../loci/formats/dicom/DCDumpProvider.java | 4 +- .../loci/formats/dicom/DicomJSONProvider.java | 101 ++++++++++++++++++ .../src/loci/formats/out/DicomWriter.java | 15 ++- 4 files changed, 122 insertions(+), 5 deletions(-) create mode 100644 components/formats-bsd/src/loci/formats/dicom/DicomJSONProvider.java diff --git a/components/formats-bsd/pom.xml b/components/formats-bsd/pom.xml index f3793031300..5a28eb66c39 100644 --- a/components/formats-bsd/pom.xml +++ b/components/formats-bsd/pom.xml @@ -118,6 +118,13 @@ ${jxrlib.version} + + org.json + json + 20230227 + + + xerces diff --git a/components/formats-bsd/src/loci/formats/dicom/DCDumpProvider.java b/components/formats-bsd/src/loci/formats/dicom/DCDumpProvider.java index fe2db38b5eb..cbfcdee2cda 100644 --- a/components/formats-bsd/src/loci/formats/dicom/DCDumpProvider.java +++ b/components/formats-bsd/src/loci/formats/dicom/DCDumpProvider.java @@ -36,7 +36,6 @@ import java.util.ArrayList; import java.util.List; -import loci.common.Constants; import loci.common.DataTools; /** @@ -75,9 +74,8 @@ else if (tokens[i].startsWith("<")) { int tagUpper = Integer.parseInt(tag[0], 16); int tagLower = Integer.parseInt(tag[1], 16); - int vrCode = DataTools.bytesToShort(vr.getBytes(Constants.ENCODING), false); - DicomTag t = new DicomTag(DicomAttribute.get(tagUpper << 16 | tagLower), DicomVR.get(vrCode)); + DicomTag t = new DicomTag(DicomAttribute.get(tagUpper << 16 | tagLower), DicomVR.valueOf(DicomVR.class, vr)); // TODO: fix value handling for more complex cases (e.g. arrays) t.value = value; tags.add(t); diff --git a/components/formats-bsd/src/loci/formats/dicom/DicomJSONProvider.java b/components/formats-bsd/src/loci/formats/dicom/DicomJSONProvider.java new file mode 100644 index 00000000000..b5763dcb35d --- /dev/null +++ b/components/formats-bsd/src/loci/formats/dicom/DicomJSONProvider.java @@ -0,0 +1,101 @@ +/* + * #%L + * BSD implementations of Bio-Formats readers and writers + * %% + * Copyright (C) 2005 - 2023 Open Microscopy Environment: + * - Board of Regents of the University of Wisconsin-Madison + * - Glencoe Software, Inc. + * - University of Dundee + * %% + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + * #L% + */ + +package loci.formats.dicom; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + +import loci.common.Constants; +import loci.common.DataTools; + +import org.json.JSONArray; +import org.json.JSONException; +import org.json.JSONObject; + +/** + * Provide DICOM tags from a file containing JSON. + * Formal JSON schema yet to be determined, but the idea is to accept a hierarchy + * of tags of the form: + * + * { + * "BodyPartExamined": { + * "Value": "BRAIN", + * "VR": "CS", + * "Tag": "(0018,0015)" + * } + * } + * + * This is similar to JSON examples in https://github.com/QIICR/dcmqi/tree/master/doc/examples, + * but allows for the VR (type) and tag to be explicitly stated, in addition to + * the more human-readable description. + */ +public class DicomJSONProvider implements ITagProvider { + + private List tags = new ArrayList(); + + @Override + public void readTagSource(String location) throws IOException { + String rawJSON = DataTools.readFile(location); + try { + JSONObject root = new JSONObject(rawJSON); + + // TODO: nested tags not currently supported + + for (String tagKey : root.keySet()) { + JSONObject tag = root.getJSONObject(tagKey); + String vr = tag.getString("VR"); + String value = tag.getString("Value"); + String[] tagCode = tag.getString("Tag").replaceAll("[()]", "").split(","); + + int tagUpper = Integer.parseInt(tagCode[0], 16); + int tagLower = Integer.parseInt(tagCode[1], 16); + + DicomAttribute attr = DicomAttribute.get(tagUpper << 16 | tagLower); + DicomVR vrEnum = DicomVR.valueOf(DicomVR.class, vr); + DicomTag dicomTag = new DicomTag(attr, vrEnum); + dicomTag.value = value; + tags.add(dicomTag); + } + } + catch (JSONException e) { + throw new IOException("Could not parse JSON", e); + } + } + + @Override + public List getTags() { + return tags; + } + +} diff --git a/components/formats-bsd/src/loci/formats/out/DicomWriter.java b/components/formats-bsd/src/loci/formats/out/DicomWriter.java index 182ecf1a41e..db6504188e3 100644 --- a/components/formats-bsd/src/loci/formats/out/DicomWriter.java +++ b/components/formats-bsd/src/loci/formats/out/DicomWriter.java @@ -58,6 +58,7 @@ import loci.formats.codec.JPEG2000CodecOptions; import loci.formats.codec.JPEGCodec; import loci.formats.dicom.DCDumpProvider; +import loci.formats.dicom.DicomJSONProvider; import loci.formats.dicom.ITagProvider; import loci.formats.dicom.DicomTag; import loci.formats.in.DynamicMetadataOptions; @@ -130,10 +131,12 @@ public void setExtraMetadata(String tagSource) { if (tagSource != null) { ITagProvider provider = null; - if (checkSuffix(tagSource, "dcdump")) { + if (checkSuffix(tagSource, "json")) { + provider = new DicomJSONProvider(); + } + else if (checkSuffix(tagSource, "dcdump")) { provider = new DCDumpProvider(); } - // TODO: allow for JSON parser here try { provider.readTagSource(tagSource); @@ -931,6 +934,14 @@ public void setId(String id) throws FormatException, IOException { for (DicomTag t : provider.getTags()) { boolean validTag = t.validate(tags); if (validTag) { + if (t.value instanceof String) { + if (t.vr == UI) { + t.value = padUID((String) t.value); + } + else { + t.value = padString((String) t.value); + } + } tags.add(t); } else { From c71011b0a5a4fee2bed504e7ca1f265181994c84 Mon Sep 17 00:00:00 2001 From: Melissa Linkert Date: Thu, 8 Jun 2023 21:46:36 -0500 Subject: [PATCH 05/18] Fix up some string parsing --- .../loci/formats/dicom/DCDumpProvider.java | 39 ++++- .../loci/formats/dicom/DicomJSONProvider.java | 8 + .../src/loci/formats/dicom/DicomTag.java | 143 ++++++++++++++++++ 3 files changed, 182 insertions(+), 8 deletions(-) diff --git a/components/formats-bsd/src/loci/formats/dicom/DCDumpProvider.java b/components/formats-bsd/src/loci/formats/dicom/DCDumpProvider.java index cbfcdee2cda..ddd09312436 100644 --- a/components/formats-bsd/src/loci/formats/dicom/DCDumpProvider.java +++ b/components/formats-bsd/src/loci/formats/dicom/DCDumpProvider.java @@ -38,24 +38,35 @@ import loci.common.DataTools; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + /** * Provide DICOM tags from a file containing the output of dcdump. */ public class DCDumpProvider implements ITagProvider { + private static final Logger LOGGER = + LoggerFactory.getLogger(DCDumpProvider.class); + private List tags = new ArrayList(); @Override public void readTagSource(String location) throws IOException { - String[] lines = DataTools.readFile(location).split("\r\n"); + String[] lines = DataTools.readFile(location).split("[\r\n]"); + LOGGER.debug("Attempting to parse {} tags", lines.length); // TODO: does not currently handle tag hierarchies for (String line : lines) { + LOGGER.debug("Parsing line: {}", line); String[] tokens = line.split(" "); - String[] tag = tokens[0].replaceAll("()", "").split(","); + String[] tag = tokens[0].replaceAll("[()]", "").split(","); - String vr = tokens[1]; - vr = vr.substring(vr.indexOf("<") + 1, vr.indexOf(">")); + String vr = tokens[1].trim(); + if (vr.isEmpty()) { + LOGGER.debug("VR not found for tag {}", tokens[0]); + continue; + } String length = null; String value = ""; @@ -69,15 +80,27 @@ else if (tokens[i].startsWith("<")) { i++; value += tokens[i]; } + value = value.substring(1, value.length() - 1); + } + else if (tokens[i].startsWith("[")) { + value += tokens[i]; + while (!value.endsWith("]")) { + i++; + value += tokens[i]; + } + value = value.substring(1, value.length() - 1); } } - int tagUpper = Integer.parseInt(tag[0], 16); - int tagLower = Integer.parseInt(tag[1], 16); + int tagUpper = Integer.decode(tag[0]); + int tagLower = Integer.decode(tag[1]); - DicomTag t = new DicomTag(DicomAttribute.get(tagUpper << 16 | tagLower), DicomVR.valueOf(DicomVR.class, vr)); - // TODO: fix value handling for more complex cases (e.g. arrays) + DicomAttribute attr = DicomAttribute.get(tagUpper << 16 | tagLower); + DicomVR vrEnum = DicomVR.valueOf(DicomVR.class, vr); + DicomTag t = new DicomTag(attr, vrEnum); t.value = value; + t.validateValue(); + LOGGER.debug("Adding tag: {}, VR: {}, value: {}", attr, vrEnum, value); tags.add(t); } } diff --git a/components/formats-bsd/src/loci/formats/dicom/DicomJSONProvider.java b/components/formats-bsd/src/loci/formats/dicom/DicomJSONProvider.java index b5763dcb35d..58c65e4809e 100644 --- a/components/formats-bsd/src/loci/formats/dicom/DicomJSONProvider.java +++ b/components/formats-bsd/src/loci/formats/dicom/DicomJSONProvider.java @@ -43,6 +43,9 @@ import org.json.JSONException; import org.json.JSONObject; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + /** * Provide DICOM tags from a file containing JSON. * Formal JSON schema yet to be determined, but the idea is to accept a hierarchy @@ -62,6 +65,9 @@ */ public class DicomJSONProvider implements ITagProvider { + private static final Logger LOGGER = + LoggerFactory.getLogger(DicomJSONProvider.class); + private List tags = new ArrayList(); @Override @@ -85,6 +91,8 @@ public void readTagSource(String location) throws IOException { DicomVR vrEnum = DicomVR.valueOf(DicomVR.class, vr); DicomTag dicomTag = new DicomTag(attr, vrEnum); dicomTag.value = value; + LOGGER.debug("Adding tag: {}, VR: {}, value: {}", attr, vrEnum, value); + dicomTag.validateValue(); tags.add(dicomTag); } } diff --git a/components/formats-bsd/src/loci/formats/dicom/DicomTag.java b/components/formats-bsd/src/loci/formats/dicom/DicomTag.java index 3714cdedb10..565f394aaf0 100644 --- a/components/formats-bsd/src/loci/formats/dicom/DicomTag.java +++ b/components/formats-bsd/src/loci/formats/dicom/DicomTag.java @@ -539,8 +539,151 @@ public boolean validate(List tags) { return true; } + /** + * Compare the value type to the VR. + * If the two types do not match, attempt to conver the value + * into the VR's type. + * Usually this means parsing numerical values from a String. + */ + public void validateValue() { + if (value == null) { + return; + } + switch (vr) { + case AE: + case AS: + case CS: + case DA: + case DS: + case DT: + case IS: + case LO: + case LT: + case PN: + case SH: + case ST: + case TM: + case UC: + case UI: + case UR: + case UT: + value = value.toString(); + break; + case AT: + case SS: + case US: + if (value instanceof Short) { + value = new short[] {(short) value}; + } + else if (value instanceof String) { + String[] values = ((String) value).split(","); + short[] v = new short[values.length]; + for (int i=0; i"); + if (sequenceElement) { + line = line.substring(1).trim(); + if (parentTag == null) { + parentTag = tags.get(tags.size() - 1); + } + } + String[] tokens = line.split(" "); + if (tokens.length <= 1) { + continue; + } String[] tag = tokens[0].replaceAll("[()]", "").split(","); String vr = tokens[1].trim(); @@ -95,13 +114,24 @@ else if (tokens[i].startsWith("[")) { int tagUpper = Integer.decode(tag[0]); int tagLower = Integer.decode(tag[1]); - DicomAttribute attr = DicomAttribute.get(tagUpper << 16 | tagLower); DicomVR vrEnum = DicomVR.valueOf(DicomVR.class, vr); - DicomTag t = new DicomTag(attr, vrEnum); - t.value = value; - t.validateValue(); - LOGGER.debug("Adding tag: {}, VR: {}, value: {}", attr, vrEnum, value); - tags.add(t); + int tagCode = tagUpper << 16 | tagLower; + DicomTag t = new DicomTag(tagCode, vrEnum); + if (vrEnum != DicomVR.SQ) { + t.value = value; + t.validateValue(); + } + LOGGER.debug("Adding tag: {}, VR: {}, value: {}", tagCode, vrEnum, value); + if (parentTag == null) { + tags.add(t); + } + else { + t.parent = parentTag; + parentTag.children.add(t); + } + if (vrEnum == DicomVR.SQ) { + parentTag = t; + } } } diff --git a/components/formats-bsd/src/loci/formats/dicom/DicomTag.java b/components/formats-bsd/src/loci/formats/dicom/DicomTag.java index 565f394aaf0..0b36b04c018 100644 --- a/components/formats-bsd/src/loci/formats/dicom/DicomTag.java +++ b/components/formats-bsd/src/loci/formats/dicom/DicomTag.java @@ -80,6 +80,17 @@ public DicomTag(DicomAttribute attribute, DicomVR vr) { this.tag = attribute.getTag(); } + public DicomTag(int tag, DicomVR vr) { + this.tag = tag; + this.attribute = DicomAttribute.get(tag); + if (vr != null) { + this.vr = vr; + } + else if (attribute != null) { + this.vr = attribute.getDefaultVR(); + } + } + /** * Read a complete tag and value from the given input stream. */ diff --git a/components/formats-bsd/src/loci/formats/out/DicomWriter.java b/components/formats-bsd/src/loci/formats/out/DicomWriter.java index db6504188e3..21dbdc52128 100644 --- a/components/formats-bsd/src/loci/formats/out/DicomWriter.java +++ b/components/formats-bsd/src/loci/formats/out/DicomWriter.java @@ -934,14 +934,7 @@ public void setId(String id) throws FormatException, IOException { for (DicomTag t : provider.getTags()) { boolean validTag = t.validate(tags); if (validTag) { - if (t.value instanceof String) { - if (t.vr == UI) { - t.value = padUID((String) t.value); - } - else { - t.value = padString((String) t.value); - } - } + padTagValues(t); tags.add(t); } else { @@ -954,7 +947,10 @@ public void setId(String id) throws FormatException, IOException { tags.sort(new Comparator() { public int compare(DicomTag a, DicomTag b) { - return a.attribute.getTag() - b.attribute.getTag(); + int aTag = a.attribute == null ? a.tag : a.attribute.getTag(); + int bTag = b.attribute == null ? b.tag : b.attribute.getTag(); + + return aTag - bTag; } }); @@ -1043,7 +1039,7 @@ private int getStoredLength(DicomTag tag) { } private void writeTag(DicomTag tag) throws IOException { - int tagCode = tag.attribute.getTag(); + int tagCode = tag.attribute == null ? tag.tag : tag.attribute.getTag(); out.writeShort((short) ((tagCode & 0xffff0000) >> 16)); out.writeShort((short) (tagCode & 0xffff)); @@ -1503,6 +1499,21 @@ private DicomTag makeItemDelimitation() { return item; } + private void padTagValues(DicomTag t) { + if (t.value instanceof String) { + if (t.vr == UI) { + t.value = padUID((String) t.value); + } + else { + t.value = padString((String) t.value); + } + } + + for (DicomTag child : t.children) { + padTagValues(child); + } + } + private short[] makeShortArray(int v) { short[] s = new short[2]; s[0] = (short) ((v >> 16) & 0xffff); From c5b34279f54507d5cbd115f95c56542bccbeecb6 Mon Sep 17 00:00:00 2001 From: Melissa Linkert Date: Fri, 9 Jun 2023 18:37:00 -0500 Subject: [PATCH 07/18] Allow tag hierarchies in DICOM JSON provider --- .../loci/formats/dicom/DicomJSONProvider.java | 58 +++++++++++++++++-- 1 file changed, 52 insertions(+), 6 deletions(-) diff --git a/components/formats-bsd/src/loci/formats/dicom/DicomJSONProvider.java b/components/formats-bsd/src/loci/formats/dicom/DicomJSONProvider.java index 58c65e4809e..1a3f5a982c8 100644 --- a/components/formats-bsd/src/loci/formats/dicom/DicomJSONProvider.java +++ b/components/formats-bsd/src/loci/formats/dicom/DicomJSONProvider.java @@ -57,6 +57,22 @@ * "VR": "CS", * "Tag": "(0018,0015)" * } + * "ContributingEquipmentSequence": { + * "VR": "SQ", + * "Tag": "(0018,a001)", + * "Sequence": { + * "Manufacturer": { + * "Value": "PixelMed", + * "VR": "LO", + * "Tag": "(0008,0070)" + * }, + * "ContributionDateTime": { + * "Value": "20210710234601.105+0000", + * "VR": "DT", + * "Tag": "(0018,a002)" + * } + * } + * } * } * * This is similar to JSON examples in https://github.com/QIICR/dcmqi/tree/master/doc/examples, @@ -76,23 +92,26 @@ public void readTagSource(String location) throws IOException { try { JSONObject root = new JSONObject(rawJSON); - // TODO: nested tags not currently supported - for (String tagKey : root.keySet()) { JSONObject tag = root.getJSONObject(tagKey); String vr = tag.getString("VR"); - String value = tag.getString("Value"); + String value = tag.has("Value") ? tag.getString("Value") : null; String[] tagCode = tag.getString("Tag").replaceAll("[()]", "").split(","); int tagUpper = Integer.parseInt(tagCode[0], 16); int tagLower = Integer.parseInt(tagCode[1], 16); - DicomAttribute attr = DicomAttribute.get(tagUpper << 16 | tagLower); + int intTagCode = tagUpper << 16 | tagLower; DicomVR vrEnum = DicomVR.valueOf(DicomVR.class, vr); - DicomTag dicomTag = new DicomTag(attr, vrEnum); + DicomTag dicomTag = new DicomTag(intTagCode, vrEnum); dicomTag.value = value; - LOGGER.debug("Adding tag: {}, VR: {}, value: {}", attr, vrEnum, value); + LOGGER.debug("Adding tag: {}, VR: {}, value: {}", intTagCode, vrEnum, value); dicomTag.validateValue(); + + if (vrEnum == DicomVR.SQ && tag.has("Sequence")) { + readSequence(tag, dicomTag); + } + tags.add(dicomTag); } } @@ -106,4 +125,31 @@ public List getTags() { return tags; } + private void readSequence(JSONObject rootTag, DicomTag parent) { + JSONObject sequence = rootTag.getJSONObject("Sequence"); + for (String key : sequence.keySet()) { + JSONObject tag = sequence.getJSONObject(key); + String vr = tag.getString("VR"); + String value = tag.getString("Value"); + String[] tagCode = tag.getString("Tag").replaceAll("[()]", "").split(","); + + int tagUpper = Integer.parseInt(tagCode[0], 16); + int tagLower = Integer.parseInt(tagCode[1], 16); + + int intTagCode = tagUpper << 16 | tagLower; + DicomVR vrEnum = DicomVR.valueOf(DicomVR.class, vr); + DicomTag dicomTag = new DicomTag(intTagCode, vrEnum); + dicomTag.value = value; + LOGGER.debug("Adding tag: {}, VR: {}, value: {}", intTagCode, vrEnum, value); + dicomTag.validateValue(); + + dicomTag.parent = parent; + parent.children.add(dicomTag); + + if (vrEnum == DicomVR.SQ && tag.get("Sequence") != null) { + readSequence(tag, dicomTag); + } + } + } + } From c03a28dcc955cb14f89b714e79ee6f8ece0a4cbd Mon Sep 17 00:00:00 2001 From: Melissa Linkert Date: Tue, 18 Jul 2023 16:30:57 -0500 Subject: [PATCH 08/18] Remove dcdump metadata provider --- .../loci/formats/dicom/DCDumpProvider.java | 143 ------------------ .../src/loci/formats/out/DicomWriter.java | 5 +- 2 files changed, 2 insertions(+), 146 deletions(-) delete mode 100644 components/formats-bsd/src/loci/formats/dicom/DCDumpProvider.java diff --git a/components/formats-bsd/src/loci/formats/dicom/DCDumpProvider.java b/components/formats-bsd/src/loci/formats/dicom/DCDumpProvider.java deleted file mode 100644 index 73487fc3c08..00000000000 --- a/components/formats-bsd/src/loci/formats/dicom/DCDumpProvider.java +++ /dev/null @@ -1,143 +0,0 @@ -/* - * #%L - * BSD implementations of Bio-Formats readers and writers - * %% - * Copyright (C) 2005 - 2023 Open Microscopy Environment: - * - Board of Regents of the University of Wisconsin-Madison - * - Glencoe Software, Inc. - * - University of Dundee - * %% - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are met: - * - * 1. Redistributions of source code must retain the above copyright notice, - * this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright notice, - * this list of conditions and the following disclaimer in the documentation - * and/or other materials provided with the distribution. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" - * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE - * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR CONTRIBUTORS BE - * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR - * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF - * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS - * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN - * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) - * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE - * POSSIBILITY OF SUCH DAMAGE. - * #L% - */ - -package loci.formats.dicom; - -import java.io.IOException; -import java.util.ArrayList; -import java.util.List; - -import loci.common.DataTools; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * Provide DICOM tags from a file containing the output of dcdump. - */ -public class DCDumpProvider implements ITagProvider { - - private static final Logger LOGGER = - LoggerFactory.getLogger(DCDumpProvider.class); - - private List tags = new ArrayList(); - - @Override - public void readTagSource(String location) throws IOException { - String[] lines = DataTools.readFile(location).split("[\r\n]"); - - LOGGER.debug("Attempting to parse {} tags", lines.length); - DicomTag parentTag = null; - for (String line : lines) { - line = line.trim(); - - if (line.isEmpty() && parentTag != null) { - // indicates end of sequence - parentTag = parentTag.parent; - continue; - } - - LOGGER.debug("Parsing line: {}", line); - boolean sequenceElement = line.startsWith(">"); - if (sequenceElement) { - line = line.substring(1).trim(); - if (parentTag == null) { - parentTag = tags.get(tags.size() - 1); - } - } - - String[] tokens = line.split(" "); - if (tokens.length <= 1) { - continue; - } - String[] tag = tokens[0].replaceAll("[()]", "").split(","); - - String vr = tokens[1].trim(); - if (vr.isEmpty()) { - LOGGER.debug("VR not found for tag {}", tokens[0]); - continue; - } - - String length = null; - String value = ""; - for (int i=2; i")) { - i++; - value += tokens[i]; - } - value = value.substring(1, value.length() - 1); - } - else if (tokens[i].startsWith("[")) { - value += tokens[i]; - while (!value.endsWith("]")) { - i++; - value += tokens[i]; - } - value = value.substring(1, value.length() - 1); - } - } - - int tagUpper = Integer.decode(tag[0]); - int tagLower = Integer.decode(tag[1]); - - DicomVR vrEnum = DicomVR.valueOf(DicomVR.class, vr); - int tagCode = tagUpper << 16 | tagLower; - DicomTag t = new DicomTag(tagCode, vrEnum); - if (vrEnum != DicomVR.SQ) { - t.value = value; - t.validateValue(); - } - LOGGER.debug("Adding tag: {}, VR: {}, value: {}", tagCode, vrEnum, value); - if (parentTag == null) { - tags.add(t); - } - else { - t.parent = parentTag; - parentTag.children.add(t); - } - if (vrEnum == DicomVR.SQ) { - parentTag = t; - } - } - } - - @Override - public List getTags() { - return tags; - } - -} diff --git a/components/formats-bsd/src/loci/formats/out/DicomWriter.java b/components/formats-bsd/src/loci/formats/out/DicomWriter.java index 21dbdc52128..128f916a96a 100644 --- a/components/formats-bsd/src/loci/formats/out/DicomWriter.java +++ b/components/formats-bsd/src/loci/formats/out/DicomWriter.java @@ -57,7 +57,6 @@ import loci.formats.codec.JPEG2000Codec; import loci.formats.codec.JPEG2000CodecOptions; import loci.formats.codec.JPEGCodec; -import loci.formats.dicom.DCDumpProvider; import loci.formats.dicom.DicomJSONProvider; import loci.formats.dicom.ITagProvider; import loci.formats.dicom.DicomTag; @@ -134,8 +133,8 @@ public void setExtraMetadata(String tagSource) { if (checkSuffix(tagSource, "json")) { provider = new DicomJSONProvider(); } - else if (checkSuffix(tagSource, "dcdump")) { - provider = new DCDumpProvider(); + else { + throw new IllegalArgumentException("Unknown tag format: " + tagSource); } try { From cc3549ed1bc26ee839bef59ef00e926908b57aea Mon Sep 17 00:00:00 2001 From: Melissa Linkert Date: Thu, 20 Jul 2023 11:07:27 -0500 Subject: [PATCH 09/18] Add configurable tag resolution strategy Each tag contained in a JSON file may now optionally contain a "ResolutionStrategy" property set to "IGNORE", "REPLACE", or "APPEND". IGNORE means that the tag will be ignored if the same tag code has been defined already. REPLACE means that the tag will be used to replace any existing tag with the same code. APPEND means that if there is an existing tag with the same code, the current tag's value will be appended to the pre-existing tag's value array. --- .../loci/formats/dicom/DicomJSONProvider.java | 7 ++++ .../src/loci/formats/dicom/DicomTag.java | 9 ++-- .../formats/dicom/ResolutionStrategy.java | 41 +++++++++++++++++++ .../src/loci/formats/out/DicomWriter.java | 36 +++++++++++++++- 4 files changed, 88 insertions(+), 5 deletions(-) create mode 100644 components/formats-bsd/src/loci/formats/dicom/ResolutionStrategy.java diff --git a/components/formats-bsd/src/loci/formats/dicom/DicomJSONProvider.java b/components/formats-bsd/src/loci/formats/dicom/DicomJSONProvider.java index 1a3f5a982c8..4301b8d2a30 100644 --- a/components/formats-bsd/src/loci/formats/dicom/DicomJSONProvider.java +++ b/components/formats-bsd/src/loci/formats/dicom/DicomJSONProvider.java @@ -112,6 +112,13 @@ public void readTagSource(String location) throws IOException { readSequence(tag, dicomTag); } + ResolutionStrategy rs = vrEnum == DicomVR.SQ ? ResolutionStrategy.APPEND : ResolutionStrategy.REPLACE; + if (tag.has("ResolutionStrategy")) { + String strategy = tag.getString("ResolutionStrategy"); + rs = Enum.valueOf(ResolutionStrategy.class, strategy); + } + dicomTag.strategy = rs; + tags.add(dicomTag); } } diff --git a/components/formats-bsd/src/loci/formats/dicom/DicomTag.java b/components/formats-bsd/src/loci/formats/dicom/DicomTag.java index 0b36b04c018..bd22fa838ef 100644 --- a/components/formats-bsd/src/loci/formats/dicom/DicomTag.java +++ b/components/formats-bsd/src/loci/formats/dicom/DicomTag.java @@ -65,6 +65,10 @@ public class DicomTag { private boolean bigEndianTransferSyntax = false; private boolean oddLocations = false; + // optional indicator of how to handle this tag when merging + // into an existing tag list + public ResolutionStrategy strategy = null; + public DicomTag(DicomAttribute attribute) { this(attribute, null); } @@ -539,12 +543,9 @@ public DicomTag lookupChild(DicomAttribute attr) { * return false. Otherwise return true. */ public boolean validate(List tags) { - // TODO: this is very simplistic validation that just rejects duplicates - // we could pursue more comprehensive validation here, - // but need to define what that means for (DicomTag t : tags) { if (this.tag == t.tag) { - return false; + return strategy != ResolutionStrategy.IGNORE; } } return true; diff --git a/components/formats-bsd/src/loci/formats/dicom/ResolutionStrategy.java b/components/formats-bsd/src/loci/formats/dicom/ResolutionStrategy.java new file mode 100644 index 00000000000..e4b8939b059 --- /dev/null +++ b/components/formats-bsd/src/loci/formats/dicom/ResolutionStrategy.java @@ -0,0 +1,41 @@ +/* + * #%L + * BSD implementations of Bio-Formats readers and writers + * %% + * Copyright (C) 2005 - 2023 Open Microscopy Environment: + * - Board of Regents of the University of Wisconsin-Madison + * - Glencoe Software, Inc. + * - University of Dundee + * %% + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + * #L% + */ + +package loci.formats.dicom; + +/** + */ +public enum ResolutionStrategy { + IGNORE, + REPLACE, + APPEND; +} diff --git a/components/formats-bsd/src/loci/formats/out/DicomWriter.java b/components/formats-bsd/src/loci/formats/out/DicomWriter.java index 128f916a96a..9c2bd64313a 100644 --- a/components/formats-bsd/src/loci/formats/out/DicomWriter.java +++ b/components/formats-bsd/src/loci/formats/out/DicomWriter.java @@ -41,7 +41,9 @@ import java.rmi.dgc.VMID; import java.rmi.server.UID; import java.util.ArrayList; +import java.util.Arrays; import java.util.Comparator; +import java.util.List; import loci.common.Constants; import loci.common.DataTools; @@ -57,6 +59,7 @@ import loci.formats.codec.JPEG2000Codec; import loci.formats.codec.JPEG2000CodecOptions; import loci.formats.codec.JPEGCodec; +import loci.formats.dicom.DicomAttribute; import loci.formats.dicom.DicomJSONProvider; import loci.formats.dicom.ITagProvider; import loci.formats.dicom.DicomTag; @@ -934,7 +937,29 @@ public void setId(String id) throws FormatException, IOException { boolean validTag = t.validate(tags); if (validTag) { padTagValues(t); - tags.add(t); + + LOGGER.trace("handling supplemental tag ({}) with strategy {}", t, t.strategy); + switch (t.strategy) { + case APPEND: + // TODO: doesn't handle sequences properly + tags.add(t); + break; + case IGNORE: + // ignore current tag if a matching tag already exists + DicomTag existing = lookupTag(tags, t); + if (existing == null) { + tags.add(t); + } + break; + case REPLACE: + // replace existing tag with current tag + DicomTag replace = lookupTag(tags, t); + if (replace != null) { + tags.remove(replace); + } + tags.add(t); + break; + } } else { LOGGER.warn("Ignoring tag {} from provider {}", t, provider); @@ -1498,6 +1523,15 @@ private DicomTag makeItemDelimitation() { return item; } + private DicomTag lookupTag(List tags, DicomTag compare) { + for (DicomTag t : tags) { + if (t.tag == compare.tag) { + return t; + } + } + return null; + } + private void padTagValues(DicomTag t) { if (t.value instanceof String) { if (t.vr == UI) { From fb647ddaf7ca8bb4e86f3be3437c9513453c7d0a Mon Sep 17 00:00:00 2001 From: Melissa Linkert Date: Thu, 24 Aug 2023 15:00:34 -0500 Subject: [PATCH 10/18] Fix some sorting issues and allow appending to existing sequences --- .../loci/formats/dicom/DicomJSONProvider.java | 10 ++++--- .../src/loci/formats/dicom/DicomTag.java | 10 ++++++- .../src/loci/formats/out/DicomWriter.java | 27 ++++++++++++------- 3 files changed, 33 insertions(+), 14 deletions(-) diff --git a/components/formats-bsd/src/loci/formats/dicom/DicomJSONProvider.java b/components/formats-bsd/src/loci/formats/dicom/DicomJSONProvider.java index 4301b8d2a30..1bae467de7e 100644 --- a/components/formats-bsd/src/loci/formats/dicom/DicomJSONProvider.java +++ b/components/formats-bsd/src/loci/formats/dicom/DicomJSONProvider.java @@ -137,7 +137,6 @@ private void readSequence(JSONObject rootTag, DicomTag parent) { for (String key : sequence.keySet()) { JSONObject tag = sequence.getJSONObject(key); String vr = tag.getString("VR"); - String value = tag.getString("Value"); String[] tagCode = tag.getString("Tag").replaceAll("[()]", "").split(","); int tagUpper = Integer.parseInt(tagCode[0], 16); @@ -146,8 +145,12 @@ private void readSequence(JSONObject rootTag, DicomTag parent) { int intTagCode = tagUpper << 16 | tagLower; DicomVR vrEnum = DicomVR.valueOf(DicomVR.class, vr); DicomTag dicomTag = new DicomTag(intTagCode, vrEnum); - dicomTag.value = value; - LOGGER.debug("Adding tag: {}, VR: {}, value: {}", intTagCode, vrEnum, value); + + if (tag.has("Value")) { + dicomTag.value = tag.get("Value"); + } + + LOGGER.debug("Adding tag: {}, VR: {}, value: {}", intTagCode, vrEnum, dicomTag.value); dicomTag.validateValue(); dicomTag.parent = parent; @@ -157,6 +160,7 @@ private void readSequence(JSONObject rootTag, DicomTag parent) { readSequence(tag, dicomTag); } } + parent.children.sort(null); } } diff --git a/components/formats-bsd/src/loci/formats/dicom/DicomTag.java b/components/formats-bsd/src/loci/formats/dicom/DicomTag.java index bd22fa838ef..8d87f899bd8 100644 --- a/components/formats-bsd/src/loci/formats/dicom/DicomTag.java +++ b/components/formats-bsd/src/loci/formats/dicom/DicomTag.java @@ -47,7 +47,7 @@ * Represents a complete DICOM tag, including the dictionary attribute, * actual VR, value, and any "child" tags (in the case of a sequence). */ -public class DicomTag { +public class DicomTag implements Comparable { public DicomTag parent = null; public List children = new ArrayList(); @@ -699,4 +699,12 @@ public String toString() { return key + " = " + value; } + @Override + public int compareTo(DicomTag o) { + int thisTag = this.attribute == null ? this.tag : this.attribute.getTag(); + int otherTag = o.attribute == null ? o.tag : o.attribute.getTag(); + + return thisTag - otherTag; + } + } diff --git a/components/formats-bsd/src/loci/formats/out/DicomWriter.java b/components/formats-bsd/src/loci/formats/out/DicomWriter.java index 9c2bd64313a..da12018d560 100644 --- a/components/formats-bsd/src/loci/formats/out/DicomWriter.java +++ b/components/formats-bsd/src/loci/formats/out/DicomWriter.java @@ -941,8 +941,22 @@ public void setId(String id) throws FormatException, IOException { LOGGER.trace("handling supplemental tag ({}) with strategy {}", t, t.strategy); switch (t.strategy) { case APPEND: - // TODO: doesn't handle sequences properly - tags.add(t); + if (t.vr == SQ) { + DicomTag existingSequence = lookupTag(tags, t); + if (existingSequence == null) { + tags.add(t); + } + else { + existingSequence.children.add(makeItem()); + for (DicomTag child : t.children) { + existingSequence.children.add(child); + } + existingSequence.children.add(makeItemDelimitation()); + } + } + else { + tags.add(t); + } break; case IGNORE: // ignore current tag if a matching tag already exists @@ -969,14 +983,7 @@ public void setId(String id) throws FormatException, IOException { // sort tags into ascending order, then write - tags.sort(new Comparator() { - public int compare(DicomTag a, DicomTag b) { - int aTag = a.attribute == null ? a.tag : a.attribute.getTag(); - int bTag = b.attribute == null ? b.tag : b.attribute.getTag(); - - return aTag - bTag; - } - }); + tags.sort(null); for (DicomTag tag : tags) { writeTag(tag); From 52dfc64541e66d182e8ca82db6d2ff517d2a70e3 Mon Sep 17 00:00:00 2001 From: Melissa Linkert Date: Thu, 24 Aug 2023 15:22:46 -0500 Subject: [PATCH 11/18] Make sure trailing padding has even length --- components/formats-bsd/src/loci/formats/out/DicomWriter.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/components/formats-bsd/src/loci/formats/out/DicomWriter.java b/components/formats-bsd/src/loci/formats/out/DicomWriter.java index a5110bd074c..a5df732e11c 100644 --- a/components/formats-bsd/src/loci/formats/out/DicomWriter.java +++ b/components/formats-bsd/src/loci/formats/out/DicomWriter.java @@ -1204,6 +1204,10 @@ public void close() throws IOException { long fp = out.getFilePointer(); writeIFDs(resolutionIndex); long length = out.getFilePointer() - fp; + if (length % 2 == 1) { + out.writeByte(0); + length++; + } out.seek(fp - 4); out.writeInt((int) length); } From 2159cd45a6ec2d444d78b96c47fb800fe023a348 Mon Sep 17 00:00:00 2001 From: Melissa Linkert Date: Mon, 28 Aug 2023 14:51:40 -0500 Subject: [PATCH 12/18] Attempt to lookup tags by name if not specified in JSON This is a bit lenient as it will ignore case and whitespace, e.g. "Study Date", "StudyDate", and "sTuDydAte " should all resolve to 0008,0020. Also fixes a couple of small discrepancies in the dictionary. --- .../loci/formats/dicom/DicomAttribute.java | 10 +++++++- .../loci/formats/dicom/DicomJSONProvider.java | 23 +++++++++++++++---- .../loci/formats/dicom/dicom-dictionary.txt | 6 ++--- 3 files changed, 31 insertions(+), 8 deletions(-) diff --git a/components/formats-bsd/src/loci/formats/dicom/DicomAttribute.java b/components/formats-bsd/src/loci/formats/dicom/DicomAttribute.java index acc3cf26009..e270a604597 100644 --- a/components/formats-bsd/src/loci/formats/dicom/DicomAttribute.java +++ b/components/formats-bsd/src/loci/formats/dicom/DicomAttribute.java @@ -787,6 +787,8 @@ public enum DicomAttribute { private static final Map dict = new HashMap(); + private static final Map nameLookup = + new HashMap(); static { try (InputStream stream = DicomAttribute.class.getResourceAsStream("dicom-dictionary.txt")) { @@ -802,7 +804,9 @@ public enum DicomAttribute { if (tokens.length > 2) { tokens[2] = tokens[2].trim(); } - dict.put((int) Long.parseLong(tokens[0], 16), tokens); + int key = (int) Long.parseLong(tokens[0], 16); + dict.put(key, tokens); + nameLookup.put(tokens[1].replaceAll("\\s", "").toLowerCase(), key); } } catch (Exception e) { @@ -872,6 +876,10 @@ public static String getDescription(int newTag) { return null; } + public static Integer getTag(String description) { + return nameLookup.get(description.toLowerCase()); + } + /** * Lookup the attribute for the given tag. * May return null if the tag is not defined in our dictionary. diff --git a/components/formats-bsd/src/loci/formats/dicom/DicomJSONProvider.java b/components/formats-bsd/src/loci/formats/dicom/DicomJSONProvider.java index 1bae467de7e..5aac5c417d0 100644 --- a/components/formats-bsd/src/loci/formats/dicom/DicomJSONProvider.java +++ b/components/formats-bsd/src/loci/formats/dicom/DicomJSONProvider.java @@ -96,12 +96,27 @@ public void readTagSource(String location) throws IOException { JSONObject tag = root.getJSONObject(tagKey); String vr = tag.getString("VR"); String value = tag.has("Value") ? tag.getString("Value") : null; - String[] tagCode = tag.getString("Tag").replaceAll("[()]", "").split(","); - int tagUpper = Integer.parseInt(tagCode[0], 16); - int tagLower = Integer.parseInt(tagCode[1], 16); + Integer intTagCode = null; + + if (tag.has("Tag")) { + String[] tagCode = tag.getString("Tag").replaceAll("[()]", "").split(","); + + int tagUpper = Integer.parseInt(tagCode[0], 16); + int tagLower = Integer.parseInt(tagCode[1], 16); + + intTagCode = tagUpper << 16 | tagLower; + } + else { + intTagCode = DicomAttribute.getTag(tagKey); + + if (intTagCode == null) { + throw new IllegalArgumentException( + "Tag not defined and could not be determined from description '" + + tagKey + "'"); + } + } - int intTagCode = tagUpper << 16 | tagLower; DicomVR vrEnum = DicomVR.valueOf(DicomVR.class, vr); DicomTag dicomTag = new DicomTag(intTagCode, vrEnum); dicomTag.value = value; diff --git a/components/formats-bsd/src/loci/formats/dicom/dicom-dictionary.txt b/components/formats-bsd/src/loci/formats/dicom/dicom-dictionary.txt index ed5ab38f832..3d29d03f7ba 100644 --- a/components/formats-bsd/src/loci/formats/dicom/dicom-dictionary.txt +++ b/components/formats-bsd/src/loci/formats/dicom/dicom-dictionary.txt @@ -123,7 +123,7 @@ 00080124, "Mapping Resource Identification Sequence", SQ 00080201, "Timezone Offset from UTC", SH 00080220, "Responsible Group Code Sequence", SQ -00080220, "Equipment Modality", CS +00080221, "Equipment Modality", CS 00080222, "Manufacturer's Related Model Group", LO 00080300, "Private Data Element Characteristics Sequence", SQ 00080301, "Private Group Reference", US @@ -285,7 +285,7 @@ 00100221, "Genetic Modifications Sequence", SQ 00100222, "Genetic Modifications Description", UC 00100223, "Genetic Modifications Nomenclature", LO -00100229, "Genetic Modifications Sequence", SQ +00100229, "Genetic Modifications Code Sequence", SQ 00101000, "Other Patient IDs", LO 00101001, "Other Patient Names", PN 00101002, "Other Patient IDs Sequence", SQ @@ -385,7 +385,7 @@ 00142002, "Evaluator Sequence", SQ 00142004, "Evaluator Number", IS 00142006, "Evaluator Name", PN -00142006, "Evaluator Attempt", IS +00142008, "Evaluation Attempt", IS 00142012, "Indication Sequence", SQ 00142014, "Indication Number", IS 00142016, "Indication Label", SH From afcce2b88d7c781837b22f745e346ae7565d40d8 Mon Sep 17 00:00:00 2001 From: Melissa Linkert Date: Wed, 30 Aug 2023 20:55:17 -0500 Subject: [PATCH 13/18] Add VR lookup If a VR is not defined, the default is used. If a VR other than the default is defined, a warning will be shown but the user-defined VR will be used. --- .../loci/formats/dicom/DicomJSONProvider.java | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/components/formats-bsd/src/loci/formats/dicom/DicomJSONProvider.java b/components/formats-bsd/src/loci/formats/dicom/DicomJSONProvider.java index 5aac5c417d0..9041284c36c 100644 --- a/components/formats-bsd/src/loci/formats/dicom/DicomJSONProvider.java +++ b/components/formats-bsd/src/loci/formats/dicom/DicomJSONProvider.java @@ -94,7 +94,6 @@ public void readTagSource(String location) throws IOException { for (String tagKey : root.keySet()) { JSONObject tag = root.getJSONObject(tagKey); - String vr = tag.getString("VR"); String value = tag.has("Value") ? tag.getString("Value") : null; Integer intTagCode = null; @@ -117,10 +116,22 @@ public void readTagSource(String location) throws IOException { } } - DicomVR vrEnum = DicomVR.valueOf(DicomVR.class, vr); + DicomVR vrEnum = DicomAttribute.getDefaultVR(intTagCode); + if (tag.has("VR")) { + DicomVR userEnum = DicomVR.valueOf(DicomVR.class, tag.getString("VR")); + if (!vrEnum.equals(userEnum)) { + LOGGER.warn("User-defined VR ({}) for {} does not match expected VR ({})", + userEnum, DicomAttribute.formatTag(intTagCode), vrEnum); + if (userEnum != null) { + vrEnum = userEnum; + } + } + } + DicomTag dicomTag = new DicomTag(intTagCode, vrEnum); dicomTag.value = value; - LOGGER.debug("Adding tag: {}, VR: {}, value: {}", intTagCode, vrEnum, value); + LOGGER.debug("Adding tag: {}, VR: {}, value: {}", + DicomAttribute.formatTag(intTagCode), vrEnum, value); dicomTag.validateValue(); if (vrEnum == DicomVR.SQ && tag.has("Sequence")) { From 7ece5a44324826c6e2592e075f9b286b5fa1efb0 Mon Sep 17 00:00:00 2001 From: Melissa Linkert Date: Thu, 31 Aug 2023 17:48:45 -0500 Subject: [PATCH 14/18] Add some unit tests for supplemental metadata --- .../utests/dicom/ProvidedMetadataTest.java | 183 ++++++++++++++++++ .../test/loci/formats/utests/testng.xml | 6 + 2 files changed, 189 insertions(+) create mode 100644 components/formats-bsd/test/loci/formats/utests/dicom/ProvidedMetadataTest.java diff --git a/components/formats-bsd/test/loci/formats/utests/dicom/ProvidedMetadataTest.java b/components/formats-bsd/test/loci/formats/utests/dicom/ProvidedMetadataTest.java new file mode 100644 index 00000000000..1609899932a --- /dev/null +++ b/components/formats-bsd/test/loci/formats/utests/dicom/ProvidedMetadataTest.java @@ -0,0 +1,183 @@ +/* + * #%L + * BSD implementations of Bio-Formats readers and writers + * %% + * Copyright (C) 2023 Open Microscopy Environment: + * - Board of Regents of the University of Wisconsin-Madison + * - Glencoe Software, Inc. + * - University of Dundee + * %% + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + * #L% + */ + +package loci.formats.utests.dicom; + +import static org.testng.AssertJUnit.assertEquals; +import static org.testng.AssertJUnit.assertNotNull; +import static org.testng.AssertJUnit.assertTrue; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; + +import loci.common.Constants; +import loci.formats.FormatException; +import loci.formats.MetadataTools; +import loci.formats.dicom.DicomAttribute; +import loci.formats.dicom.DicomTag; +import loci.formats.dicom.DicomVR; +import loci.formats.in.DicomReader; +import loci.formats.in.FakeReader; +import loci.formats.meta.IMetadata; +import loci.formats.out.DicomWriter; + +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +/** + */ +public class ProvidedMetadataTest { + + public Path doConversion(String jsonMetadata) throws FormatException, IOException { + Path jsonFile = Files.createTempFile("dicom", ".json"); + Files.write(jsonFile, jsonMetadata.getBytes(Constants.ENCODING)); + + IMetadata meta = MetadataTools.createOMEXMLMetadata(); + FakeReader reader = new FakeReader(); + reader.setMetadataStore(meta); + reader.setId("test.fake"); + + DicomWriter writer = new DicomWriter(); + writer.setExtraMetadata(jsonFile.toString()); + writer.setMetadataRetrieve(meta); + Path dicomFile = Files.createTempFile("metadata-test", "dcm"); + writer.setId(dicomFile.toString()); + writer.saveBytes(0, reader.openBytes(0)); + + reader.close(); + writer.close(); + Files.delete(jsonFile); + + return dicomFile; + } + + public List getTags(Path dicomFile) throws FormatException, IOException { + DicomReader reader = new DicomReader(); + reader.setId(dicomFile.toString()); + List tags = reader.getTags(); + reader.close(); + return tags; + } + + public DicomTag lookup(List tags, DicomAttribute attr) { + for (DicomTag t : tags) { + if (attr.equals(t.attribute)) { + return t; + } + } + return null; + } + + @Test + public void testSingleTagWithDefaults() throws FormatException, IOException { + String json = "{" + + "\"BodyPartExamined\": {" + + "\"Value\": \"BRAIN\"" + + "}}"; + + Path dicomFile = doConversion(json); + List tags = getTags(dicomFile); + DicomTag found = lookup(tags, DicomAttribute.BODY_PART_EXAMINED); + assertNotNull(found); + + // trailing space is not a typo + // the writer pads string values to an even number of characters + assertEquals(found.value, "BRAIN "); + assertEquals(found.vr, DicomVR.CS); + + Files.delete(dicomFile); + } + + @Test(expectedExceptions={ IllegalArgumentException.class }) + public void testSingleInvalidTagName() throws FormatException, IOException { + String json = "{" + + "\"not a valid tag\": {" + + "\"Value\": \"x\"" + + "}}"; + + doConversion(json); + } + + @Test(expectedExceptions={ IllegalArgumentException.class }) + public void testSingleTagWeirdNameWithWhitespace() throws FormatException, IOException { + String json = "{" + + "\"bOdy PaRt examiNeD \": {" + + "\"Value\": \"x\"" + + "}}"; + + doConversion(json); + } + + @Test + public void testSingleTagWeirdName() throws FormatException, IOException { + String json = "{" + + "\"bOdyPaRtexamiNeD\": {" + + "\"Value\": \"BRAIN\"" + + "}}"; + + Path dicomFile = doConversion(json); + List tags = getTags(dicomFile); + DicomTag found = lookup(tags, DicomAttribute.BODY_PART_EXAMINED); + assertNotNull(found); + + // trailing space is not a typo + // the writer pads string values to an even number of characters + assertEquals(found.value, "BRAIN "); + assertEquals(found.vr, DicomVR.CS); + + Files.delete(dicomFile); + } + + @Test + public void testSingleTagCustomVR() throws FormatException, IOException { + String json = "{" + + "\"BodyPartExamined\": {" + + "\"Value\": \"0\"," + + "\"VR\": \"SH\"" + + "}}"; + + Path dicomFile = doConversion(json); + List tags = getTags(dicomFile); + DicomTag found = lookup(tags, DicomAttribute.BODY_PART_EXAMINED); + assertNotNull(found); + + assertEquals(found.value, "0 "); + assertEquals(found.vr, DicomVR.SH); + + Files.delete(dicomFile); + } + + // TODO: add some more complex examples including sequences and different ResolutionStrategy values + +} diff --git a/components/formats-bsd/test/loci/formats/utests/testng.xml b/components/formats-bsd/test/loci/formats/utests/testng.xml index 1942d83acbf..95bec190a62 100644 --- a/components/formats-bsd/test/loci/formats/utests/testng.xml +++ b/components/formats-bsd/test/loci/formats/utests/testng.xml @@ -193,4 +193,10 @@ + + + + + + From c7fd5281afd221fa6b0c78fd259c4d0240995bd9 Mon Sep 17 00:00:00 2001 From: Melissa Linkert Date: Fri, 1 Sep 2023 13:09:26 -0500 Subject: [PATCH 15/18] Expand unit tests to cover sequences and ResolutionStrategy Also fixes tag/VR lookup for sequences. --- .../loci/formats/dicom/DicomJSONProvider.java | 93 +++-- .../utests/dicom/ProvidedMetadataTest.java | 379 +++++++++++++++--- 2 files changed, 380 insertions(+), 92 deletions(-) diff --git a/components/formats-bsd/src/loci/formats/dicom/DicomJSONProvider.java b/components/formats-bsd/src/loci/formats/dicom/DicomJSONProvider.java index 9041284c36c..1dd6d4e962a 100644 --- a/components/formats-bsd/src/loci/formats/dicom/DicomJSONProvider.java +++ b/components/formats-bsd/src/loci/formats/dicom/DicomJSONProvider.java @@ -96,37 +96,8 @@ public void readTagSource(String location) throws IOException { JSONObject tag = root.getJSONObject(tagKey); String value = tag.has("Value") ? tag.getString("Value") : null; - Integer intTagCode = null; - - if (tag.has("Tag")) { - String[] tagCode = tag.getString("Tag").replaceAll("[()]", "").split(","); - - int tagUpper = Integer.parseInt(tagCode[0], 16); - int tagLower = Integer.parseInt(tagCode[1], 16); - - intTagCode = tagUpper << 16 | tagLower; - } - else { - intTagCode = DicomAttribute.getTag(tagKey); - - if (intTagCode == null) { - throw new IllegalArgumentException( - "Tag not defined and could not be determined from description '" + - tagKey + "'"); - } - } - - DicomVR vrEnum = DicomAttribute.getDefaultVR(intTagCode); - if (tag.has("VR")) { - DicomVR userEnum = DicomVR.valueOf(DicomVR.class, tag.getString("VR")); - if (!vrEnum.equals(userEnum)) { - LOGGER.warn("User-defined VR ({}) for {} does not match expected VR ({})", - userEnum, DicomAttribute.formatTag(intTagCode), vrEnum); - if (userEnum != null) { - vrEnum = userEnum; - } - } - } + Integer intTagCode = lookupTag(tagKey, tag); + DicomVR vrEnum = lookupVR(intTagCode, tag); DicomTag dicomTag = new DicomTag(intTagCode, vrEnum); dicomTag.value = value; @@ -162,14 +133,10 @@ private void readSequence(JSONObject rootTag, DicomTag parent) { JSONObject sequence = rootTag.getJSONObject("Sequence"); for (String key : sequence.keySet()) { JSONObject tag = sequence.getJSONObject(key); - String vr = tag.getString("VR"); - String[] tagCode = tag.getString("Tag").replaceAll("[()]", "").split(","); - int tagUpper = Integer.parseInt(tagCode[0], 16); - int tagLower = Integer.parseInt(tagCode[1], 16); + Integer intTagCode = lookupTag(key, tag); + DicomVR vrEnum = lookupVR(intTagCode, tag); - int intTagCode = tagUpper << 16 | tagLower; - DicomVR vrEnum = DicomVR.valueOf(DicomVR.class, vr); DicomTag dicomTag = new DicomTag(intTagCode, vrEnum); if (tag.has("Value")) { @@ -189,4 +156,56 @@ private void readSequence(JSONObject rootTag, DicomTag parent) { parent.children.sort(null); } + /** + * Get the tag code corresponding to the given JSON object. + * If "Tag" is not a defined attribute, lookup the default + * for the object name in the dictionary. + */ + private Integer lookupTag(String tagKey, JSONObject tag) { + Integer intTagCode = null; + + if (tag.has("Tag")) { + String[] tagCode = tag.getString("Tag").replaceAll("[()]", "").split(","); + + int tagUpper = Integer.parseInt(tagCode[0], 16); + int tagLower = Integer.parseInt(tagCode[1], 16); + + intTagCode = tagUpper << 16 | tagLower; + } + else { + intTagCode = DicomAttribute.getTag(tagKey); + + if (intTagCode == null) { + throw new IllegalArgumentException( + "Tag not defined and could not be determined from description '" + + tagKey + "'"); + } + } + + return intTagCode; + } + + /** + * Get the VR associated with the given tag code and JSON object. + * If "VR" is not a defined attribute, lookup the default for the + * given tag code in the dictionary. + */ + private DicomVR lookupVR(Integer intTagCode, JSONObject tag) { + DicomVR vrEnum = DicomAttribute.getDefaultVR(intTagCode); + if (tag.has("VR")) { + DicomVR userEnum = DicomVR.valueOf(DicomVR.class, tag.getString("VR")); + if (!vrEnum.equals(userEnum)) { + LOGGER.warn("User-defined VR ({}) for {} does not match expected VR ({})", + userEnum, DicomAttribute.formatTag(intTagCode), vrEnum); + if (userEnum != null) { + vrEnum = userEnum; + } + } + } + + return vrEnum; + } + + + } diff --git a/components/formats-bsd/test/loci/formats/utests/dicom/ProvidedMetadataTest.java b/components/formats-bsd/test/loci/formats/utests/dicom/ProvidedMetadataTest.java index 1609899932a..9ea0643654c 100644 --- a/components/formats-bsd/test/loci/formats/utests/dicom/ProvidedMetadataTest.java +++ b/components/formats-bsd/test/loci/formats/utests/dicom/ProvidedMetadataTest.java @@ -61,23 +61,30 @@ public class ProvidedMetadataTest { public Path doConversion(String jsonMetadata) throws FormatException, IOException { Path jsonFile = Files.createTempFile("dicom", ".json"); - Files.write(jsonFile, jsonMetadata.getBytes(Constants.ENCODING)); - - IMetadata meta = MetadataTools.createOMEXMLMetadata(); + Path dicomFile = Files.createTempFile("metadata-test", ".dcm"); FakeReader reader = new FakeReader(); - reader.setMetadataStore(meta); - reader.setId("test.fake"); - DicomWriter writer = new DicomWriter(); - writer.setExtraMetadata(jsonFile.toString()); - writer.setMetadataRetrieve(meta); - Path dicomFile = Files.createTempFile("metadata-test", "dcm"); - writer.setId(dicomFile.toString()); - writer.saveBytes(0, reader.openBytes(0)); + try { + Files.write(jsonFile, jsonMetadata.getBytes(Constants.ENCODING)); - reader.close(); - writer.close(); - Files.delete(jsonFile); + IMetadata meta = MetadataTools.createOMEXMLMetadata(); + reader.setMetadataStore(meta); + reader.setId("test.fake"); + + writer.setExtraMetadata(jsonFile.toString()); + writer.setMetadataRetrieve(meta); + writer.setId(dicomFile.toString()); + writer.saveBytes(0, reader.openBytes(0)); + } + catch (Throwable e) { + Files.delete(dicomFile); + throw e; + } + finally { + reader.close(); + writer.close(); + Files.delete(jsonFile); + } return dicomFile; } @@ -101,22 +108,26 @@ public DicomTag lookup(List tags, DicomAttribute attr) { @Test public void testSingleTagWithDefaults() throws FormatException, IOException { - String json = "{" + - "\"BodyPartExamined\": {" + - "\"Value\": \"BRAIN\"" + - "}}"; - - Path dicomFile = doConversion(json); - List tags = getTags(dicomFile); - DicomTag found = lookup(tags, DicomAttribute.BODY_PART_EXAMINED); - assertNotNull(found); + Path dicomFile = null; + try { + String json = "{" + + "\"BodyPartExamined\": {" + + "\"Value\": \"BRAIN\"" + + "}}"; - // trailing space is not a typo - // the writer pads string values to an even number of characters - assertEquals(found.value, "BRAIN "); - assertEquals(found.vr, DicomVR.CS); + dicomFile = doConversion(json); + List tags = getTags(dicomFile); + DicomTag found = lookup(tags, DicomAttribute.BODY_PART_EXAMINED); + assertNotNull(found); - Files.delete(dicomFile); + // trailing space is not a typo + // the writer pads string values to an even number of characters + assertEquals(found.value, "BRAIN "); + assertEquals(found.vr, DicomVR.CS); + } + finally { + Files.delete(dicomFile); + } } @Test(expectedExceptions={ IllegalArgumentException.class }) @@ -141,43 +152,301 @@ public void testSingleTagWeirdNameWithWhitespace() throws FormatException, IOExc @Test public void testSingleTagWeirdName() throws FormatException, IOException { - String json = "{" + - "\"bOdyPaRtexamiNeD\": {" + - "\"Value\": \"BRAIN\"" + - "}}"; - - Path dicomFile = doConversion(json); - List tags = getTags(dicomFile); - DicomTag found = lookup(tags, DicomAttribute.BODY_PART_EXAMINED); - assertNotNull(found); + Path dicomFile = null; + try { + String json = "{" + + "\"bOdyPaRtexamiNeD\": {" + + "\"Value\": \"BRAIN\"" + + "}}"; - // trailing space is not a typo - // the writer pads string values to an even number of characters - assertEquals(found.value, "BRAIN "); - assertEquals(found.vr, DicomVR.CS); + dicomFile = doConversion(json); + List tags = getTags(dicomFile); + DicomTag found = lookup(tags, DicomAttribute.BODY_PART_EXAMINED); + assertNotNull(found); - Files.delete(dicomFile); + // trailing space is not a typo + // the writer pads string values to an even number of characters + assertEquals(found.value, "BRAIN "); + assertEquals(found.vr, DicomVR.CS); + } + finally { + Files.delete(dicomFile); + } } @Test public void testSingleTagCustomVR() throws FormatException, IOException { - String json = "{" + - "\"BodyPartExamined\": {" + - "\"Value\": \"0\"," + - "\"VR\": \"SH\"" + - "}}"; + Path dicomFile = null; + try { + String json = "{" + + "\"BodyPartExamined\": {" + + "\"Value\": \"0\"," + + "\"VR\": \"SH\"" + + "}}"; + + dicomFile = doConversion(json); + List tags = getTags(dicomFile); + DicomTag found = lookup(tags, DicomAttribute.BODY_PART_EXAMINED); + assertNotNull(found); - Path dicomFile = doConversion(json); - List tags = getTags(dicomFile); - DicomTag found = lookup(tags, DicomAttribute.BODY_PART_EXAMINED); - assertNotNull(found); + assertEquals(found.value, "0 "); + assertEquals(found.vr, DicomVR.SH); + } + finally { + Files.delete(dicomFile); + } + } + + @Test + public void testReplaceExistingTag() throws FormatException, IOException { + Path dicomFile = null; + try { + String json = "{" + + "\"SpecimenLabelInImage\": {" + + "\"Value\": \"NO\"" + + "}}"; + + dicomFile = doConversion(json); + List tags = getTags(dicomFile); + DicomTag found = lookup(tags, DicomAttribute.SPECIMEN_LABEL_IN_IMAGE); + assertNotNull(found); + + assertEquals(found.value, "NO"); + assertEquals(found.vr, DicomVR.CS); + } + finally { + Files.delete(dicomFile); + } + } - assertEquals(found.value, "0 "); - assertEquals(found.vr, DicomVR.SH); + @Test + public void testIgnoreExistingTag() throws FormatException, IOException { + Path dicomFile = null; + try { + String json = "{" + + "\"SpecimenLabelInImage\": {" + + "\"Value\": \"NO\"," + + "\"ResolutionStrategy\": \"IGNORE\"" + + "}}"; + + dicomFile = doConversion(json); + List tags = getTags(dicomFile); + DicomTag found = lookup(tags, DicomAttribute.SPECIMEN_LABEL_IN_IMAGE); + assertNotNull(found); - Files.delete(dicomFile); + assertEquals(found.value, "YES "); + assertEquals(found.vr, DicomVR.CS); + } + finally { + Files.delete(dicomFile); + } } - // TODO: add some more complex examples including sequences and different ResolutionStrategy values + @Test + public void testReplaceOpticalPath() throws FormatException, IOException { + Path dicomFile = null; + try { + String json = "{" + + "\"OpticalPathSequence\": {" + + "\"Sequence\": {" + + "\"IlluminationTypeCodeSequence\": {" + + "\"Sequence\": {" + + "\"CodeValue\": {" + + "\"VR\": \"SH\"," + + "\"Value\": \"111743\"" + + "}," + + "\"CodingSchemeDesignator\": {" + + "\"VR\": \"SH\"," + + "\"Value\": \"DCM\"" + + "}," + + "\"CodeMeaning\": {" + + "\"VR\": \"LO\"," + + "\"Tag\": \"(0008,0104)\"," + + "\"Value\": \"Epifluorescence illumination\"" + + "}}}," + + "\"IlluminationWaveLength\": {" + + "\"Value\": \"488.0\"" + + "}," + + "\"OpticalPathIdentifier\": {" + + "\"Value\": \"1\"" + + "}," + + "\"OpticalPathDescription\": {" + + "\"Value\": \"replacement channel\" " + + "}}," + + "\"ResolutionStrategy\": \"REPLACE\"" + + "}}"; + + dicomFile = doConversion(json); + List tags = getTags(dicomFile); + + DicomTag found = lookup(tags, DicomAttribute.OPTICAL_PATH_SEQUENCE); + assertNotNull(found); + + assertEquals(found.children.size(), 4); + assertEquals(found.vr, DicomVR.SQ); + + assertEquals(found.children.get(0).attribute, DicomAttribute.ILLUMINATION_TYPE_CODE_SEQUENCE); + + assertEquals(found.children.get(1).attribute, DicomAttribute.ILLUMINATION_WAVELENGTH); + assertEquals(found.children.get(1).value, 488f); + + assertEquals(found.children.get(2).attribute, DicomAttribute.OPTICAL_PATH_ID); + assertEquals(found.children.get(2).value, "1 "); + + assertEquals(found.children.get(3).attribute, DicomAttribute.OPTICAL_PATH_DESCRIPTION); + assertEquals(found.children.get(3).value, "replacement channel "); + } + finally { + Files.delete(dicomFile); + } + } + + @Test + public void testAppendOpticalPath() throws FormatException, IOException { + Path dicomFile = null; + try { + String json = "{" + + "\"OpticalPathSequence\": {" + + "\"Sequence\": {" + + "\"IlluminationTypeCodeSequence\": {" + + "\"Sequence\": {" + + "\"CodeValue\": {" + + "\"VR\": \"SH\"," + + "\"Value\": \"111743\"" + + "}," + + "\"CodingSchemeDesignator\": {" + + "\"VR\": \"SH\"," + + "\"Value\": \"DCM\"" + + "}," + + "\"CodeMeaning\": {" + + "\"VR\": \"LO\"," + + "\"Tag\": \"(0008,0104)\"," + + "\"Value\": \"Epifluorescence illumination\"" + + "}}}," + + "\"IlluminationWaveLength\": {" + + "\"Value\": \"488.0\"" + + "}," + + "\"OpticalPathIdentifier\": {" + + "\"Value\": \"1\"" + + "}," + + "\"OpticalPathDescription\": {" + + "\"Value\": \"replacement channel\" " + + "}}," + + "\"ResolutionStrategy\": \"APPEND\"" + + "}}"; + + dicomFile = doConversion(json); + List tags = getTags(dicomFile); + + DicomTag found = lookup(tags, DicomAttribute.OPTICAL_PATH_SEQUENCE); + assertNotNull(found); + + assertEquals(found.children.size(), 8); + assertEquals(found.vr, DicomVR.SQ); + + assertEquals(found.children.get(0).attribute, DicomAttribute.ILLUMINATION_TYPE_CODE_SEQUENCE); + + assertEquals(found.children.get(1).attribute, DicomAttribute.ILLUMINATION_WAVELENGTH); + assertEquals(found.children.get(1).value, 1f); + + assertEquals(found.children.get(2).attribute, DicomAttribute.OPTICAL_PATH_ID); + assertEquals(found.children.get(2).value, "0 "); + + assertEquals(found.children.get(3).attribute, DicomAttribute.OPTICAL_PATH_DESCRIPTION); + assertEquals(found.children.get(3).value, ""); + + assertEquals(found.children.get(4).attribute, DicomAttribute.ILLUMINATION_TYPE_CODE_SEQUENCE); + + assertEquals(found.children.get(5).attribute, DicomAttribute.ILLUMINATION_WAVELENGTH); + assertEquals(found.children.get(5).value, 488f); + + assertEquals(found.children.get(6).attribute, DicomAttribute.OPTICAL_PATH_ID); + assertEquals(found.children.get(6).value, "1 "); + + assertEquals(found.children.get(7).attribute, DicomAttribute.OPTICAL_PATH_DESCRIPTION); + assertEquals(found.children.get(7).value, "replacement channel "); + } + finally { + Files.delete(dicomFile); + } + } + + @Test + public void testIgnoreOpticalPath() throws FormatException, IOException { + Path dicomFile = null; + try { + String json = "{" + + "\"OpticalPathSequence\": {" + + "\"Sequence\": {" + + "\"IlluminationTypeCodeSequence\": {" + + "\"Sequence\": {" + + "\"CodeValue\": {" + + "\"VR\": \"SH\"," + + "\"Value\": \"111743\"" + + "}," + + "\"CodingSchemeDesignator\": {" + + "\"VR\": \"SH\"," + + "\"Value\": \"DCM\"" + + "}," + + "\"CodeMeaning\": {" + + "\"VR\": \"LO\"," + + "\"Tag\": \"(0008,0104)\"," + + "\"Value\": \"Epifluorescence illumination\"" + + "}}}," + + "\"IlluminationWaveLength\": {" + + "\"Value\": \"488.0\"" + + "}," + + "\"OpticalPathIdentifier\": {" + + "\"Value\": \"1\"" + + "}," + + "\"OpticalPathDescription\": {" + + "\"Value\": \"replacement channel\" " + + "}}," + + "\"ResolutionStrategy\": \"IGNORE\"" + + "}}"; + + dicomFile = doConversion(json); + List tags = getTags(dicomFile); + + DicomTag found = lookup(tags, DicomAttribute.OPTICAL_PATH_SEQUENCE); + assertNotNull(found); + + assertEquals(found.children.size(), 4); + assertEquals(found.vr, DicomVR.SQ); + + assertEquals(found.children.get(0).attribute, DicomAttribute.ILLUMINATION_TYPE_CODE_SEQUENCE); + + assertEquals(found.children.get(1).attribute, DicomAttribute.ILLUMINATION_WAVELENGTH); + assertEquals(found.children.get(1).value, 1f); + + assertEquals(found.children.get(2).attribute, DicomAttribute.OPTICAL_PATH_ID); + assertEquals(found.children.get(2).value, "0 "); + + assertEquals(found.children.get(3).attribute, DicomAttribute.OPTICAL_PATH_DESCRIPTION); + assertEquals(found.children.get(3).value, ""); + } + finally { + Files.delete(dicomFile); + } + } + + @Test + public void testIgnoreInvalidJSON() throws FormatException, IOException { + Path dicomFile = null; + try { + String json = "{" + + "\"BodyPartExamined\": {" + + "\"Value\": \"BRAIN\"" + + "}"; + + dicomFile = doConversion(json); + List tags = getTags(dicomFile); + DicomTag found = lookup(tags, DicomAttribute.BODY_PART_EXAMINED); + assertEquals(found, null); + } + finally { + Files.delete(dicomFile); + } + } } From 09f65ff1f1ee239a6aaadfb864b7c214804572e1 Mon Sep 17 00:00:00 2001 From: Melissa Linkert Date: Mon, 11 Sep 2023 13:06:45 -0500 Subject: [PATCH 16/18] Temporarily comment out all but one test, for build failure investigation --- .../test/loci/formats/utests/dicom/ProvidedMetadataTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/components/formats-bsd/test/loci/formats/utests/dicom/ProvidedMetadataTest.java b/components/formats-bsd/test/loci/formats/utests/dicom/ProvidedMetadataTest.java index 9ea0643654c..f67553c449f 100644 --- a/components/formats-bsd/test/loci/formats/utests/dicom/ProvidedMetadataTest.java +++ b/components/formats-bsd/test/loci/formats/utests/dicom/ProvidedMetadataTest.java @@ -130,6 +130,7 @@ public void testSingleTagWithDefaults() throws FormatException, IOException { } } + /* @Test(expectedExceptions={ IllegalArgumentException.class }) public void testSingleInvalidTagName() throws FormatException, IOException { String json = "{" + @@ -448,5 +449,6 @@ public void testIgnoreInvalidJSON() throws FormatException, IOException { Files.delete(dicomFile); } } + */ } From 5a787f6795ab1e6dac92a5ffb7c0d770de866a44 Mon Sep 17 00:00:00 2001 From: Melissa Linkert Date: Mon, 11 Sep 2023 15:03:33 -0500 Subject: [PATCH 17/18] Revert "Temporarily comment out all but one test, for build failure investigation" This reverts commit 09f65ff1f1ee239a6aaadfb864b7c214804572e1. --- .../test/loci/formats/utests/dicom/ProvidedMetadataTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/components/formats-bsd/test/loci/formats/utests/dicom/ProvidedMetadataTest.java b/components/formats-bsd/test/loci/formats/utests/dicom/ProvidedMetadataTest.java index f67553c449f..9ea0643654c 100644 --- a/components/formats-bsd/test/loci/formats/utests/dicom/ProvidedMetadataTest.java +++ b/components/formats-bsd/test/loci/formats/utests/dicom/ProvidedMetadataTest.java @@ -130,7 +130,6 @@ public void testSingleTagWithDefaults() throws FormatException, IOException { } } - /* @Test(expectedExceptions={ IllegalArgumentException.class }) public void testSingleInvalidTagName() throws FormatException, IOException { String json = "{" + @@ -449,6 +448,5 @@ public void testIgnoreInvalidJSON() throws FormatException, IOException { Files.delete(dicomFile); } } - */ } From caccc630b6056500bc36a988fb85367fd5c8bac5 Mon Sep 17 00:00:00 2001 From: Melissa Linkert Date: Mon, 11 Sep 2023 15:04:22 -0500 Subject: [PATCH 18/18] Turn off file grouping in test step that reads converted DICOM files This prevents test run times from depending on the contents of the temp directory. Without this change, some builds failed as the tests didn't finish. --- .../test/loci/formats/utests/dicom/ProvidedMetadataTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/components/formats-bsd/test/loci/formats/utests/dicom/ProvidedMetadataTest.java b/components/formats-bsd/test/loci/formats/utests/dicom/ProvidedMetadataTest.java index 9ea0643654c..e0e0cc63805 100644 --- a/components/formats-bsd/test/loci/formats/utests/dicom/ProvidedMetadataTest.java +++ b/components/formats-bsd/test/loci/formats/utests/dicom/ProvidedMetadataTest.java @@ -91,6 +91,7 @@ public Path doConversion(String jsonMetadata) throws FormatException, IOExceptio public List getTags(Path dicomFile) throws FormatException, IOException { DicomReader reader = new DicomReader(); + reader.setGroupFiles(false); reader.setId(dicomFile.toString()); List tags = reader.getTags(); reader.close();