Repository: pydicom/pydicom

Issue:
DataElement_from_raw exception in 0.9.8
_From [barlen...@gmail.com](https://code.google.com/u/100416846374427381147/) on February 05, 2013 10:59:59_

Hi,

I have written a module to hold the tags from a set of dicom files which worked with pydicom 0.9.7.  (Thanks!)  With 0.9.8,

value = dicom.dataelem.DataElement_from_raw(value)

throws (with debugging info appended):

TypeError: "'NoneType' object is unsubscriptable"

> /usr/lib/python2.6/site-packages/dicom/values.py(191)convert_value()
>     190         # Text VRs use the 2nd specified encoding

--> 191         value = converter(byte_string, is_little_endian, encoding=encoding[1])
    192     elif VR != "SQ":

ipdb> byte_string
'<RegExProg>head\ARIC_NCS_HUMAN_3T_SKYRA_VD11_D_MRQ\32 coil Human</RegExProg>'
ipdb> is_little_endian
True
ipdb> encoding
ipdb> default_encoding
'iso8859'
ipdb> encoding
ipdb> u

> /usr/lib/python2.6/site-packages/dicom/dataelem.py(338)DataElement_from_raw()
>     337     try:
> 2-> 338         value = convert_value(VR, raw, encoding)
>     339     except NotImplementedError as e:

ipdb> encoding
ipdb> l
    333             elif raw.tag.element == 0:  # group length tag implied in versions < 3.0
    334                 VR = 'UL'
    335             else:
    336                 raise KeyError("Unknown DICOM tag {0:s} - can't look up VR".format(str(raw.tag)))
    337     try:
2-> 338         value = convert_value(VR, raw, encoding)
    339     except NotImplementedError as e:
    340         raise NotImplementedError("{0:s} in tag {1!r}".format(str(e), raw.tag))
    341     return DataElement(raw.tag, VR, value, raw.value_tell,
    342                     raw.length == 0xFFFFFFFF, already_converted=True)

What happened is that I loaded a DICOM file including the tag

(0010,4000) LT [<RegExProg>head\ARIC_NCS_HUMAN_3T_SKYRA_VD11_D_MRQ\32 coil Human</... #  76, 1 PatientComments

dataelem.DataElement_from_raw's signature is 
DataElement_from_raw(raw_data_element, encoding=None) and calls values.convert_value with convert_value(VR, raw, encoding).  The default encoding for values.convert_value is not None, however, and it can't deal with it.

I worked around this in my code by changing the DataElement_from_raw call to

from dicom.charset import default_encoding
value = dicom.dataelem.DataElement_from_raw(value, default_encoding)

(not being enough of an encoding expert to know what else to use).

Thanks,

Rob Reid

_Original issue: http://code.google.com/p/pydicom/issues/detail?id=122_


Discussion:

Patch:
diff --git a/pydicom/dataelem.py b/pydicom/dataelem.py
index 4c6cc794d..36315e0ba 100644
--- a/pydicom/dataelem.py
+++ b/pydicom/dataelem.py
@@ -14,6 +14,8 @@ A DataElement has a tag,
 from __future__ import absolute_import
 from collections import namedtuple
 
+from pydicom.charset import default_encoding
+
 from pydicom import config  # don't import datetime_conversion directly
 from pydicom import compat
 from pydicom.config import logger
@@ -459,6 +461,8 @@ def DataElement_from_raw(raw_data_element, encoding=None):
     # filereader->Dataset->convert_value->filereader
     # (for SQ parsing)
 
+    if in_py2:
+        encoding = encoding or default_encoding
     from pydicom.values import convert_value
     raw = raw_data_element
 
diff --git a/pydicom/filereader.py b/pydicom/filereader.py
index b9874fc41..692cc43d9 100644
--- a/pydicom/filereader.py
+++ b/pydicom/filereader.py
@@ -14,7 +14,7 @@ import zlib
 from io import BytesIO
 
 from pydicom.misc import size_in_bytes
-from pydicom.tag import TupleTag
+from pydicom.tag import TupleTag, Tag, BaseTag
 from pydicom.dataelem import RawDataElement
 from pydicom.util.hexutil import bytes2hex
 from pydicom.valuerep import extra_length_VRs
@@ -41,7 +41,7 @@ from pydicom.dataset import (
 )
 
 from pydicom.dicomdir import DicomDir
-from pydicom.datadict import dictionary_VR
+from pydicom.datadict import dictionary_VR, tag_for_keyword
 from pydicom.dataelem import DataElement
 from pydicom.tag import (
     ItemTag,
@@ -159,7 +159,8 @@ def data_element_generator(fp,
                            is_little_endian,
                            stop_when=None,
                            defer_size=None,
-                           encoding=default_encoding):
+                           encoding=default_encoding,
+                           specific_tags=None):
 
     """Create a generator to efficiently return the raw data elements.
 
@@ -177,6 +178,8 @@ def data_element_generator(fp,
         See ``read_file`` for parameter info.
     encoding :
         Encoding scheme
+    specific_tags : list or None
+        See ``read_file`` for parameter info.
 
     Returns
     -------
@@ -230,6 +233,18 @@ def data_element_generator(fp,
     element_struct_unpack = element_struct.unpack
     defer_size = size_in_bytes(defer_size)
 
+    tag_set = set()
+    has_specific_char_set = True
+    if specific_tags is not None:
+        for tag in specific_tags:
+            if isinstance(tag, (str, compat.text_type)):
+                tag = Tag(tag_for_keyword(tag))
+            if isinstance(tag, BaseTag):
+                tag_set.add(tag)
+        has_specific_char_set = Tag(0x08, 0x05) in tag_set
+        tag_set.add(Tag(0x08, 0x05))
+    has_tag_set = len(tag_set) > 0
+
     while True:
         # Read tag, VR, length, get ready to read value
         bytes_read = fp_read(8)
@@ -282,6 +297,11 @@ def data_element_generator(fp,
         if length != 0xFFFFFFFF:
             # don't defer loading of Specific Character Set value as it is
             # needed immediately to get the character encoding for other tags
+            if has_tag_set and tag not in tag_set:
+                # skip the tag if not in specific tags
+                fp.seek(fp_tell() + length)
+                continue
+
             if defer_size is not None and length > defer_size and tag != (
                     0x08, 0x05):
                 # Flag as deferred by setting value to None, and skip bytes
@@ -309,6 +329,8 @@ def data_element_generator(fp,
                 # Store the encoding value in the generator
                 # for use with future elements (SQs)
                 encoding = convert_encodings(encoding)
+                if not has_specific_char_set:
+                    continue
 
             yield RawDataElement(tag, VR, length, value, value_tell,
                                  is_implicit_VR, is_little_endian)
@@ -340,6 +362,8 @@ def data_element_generator(fp,
                     logger_debug(msg.format(fp_tell()))
                 seq = read_sequence(fp, is_implicit_VR,
                                     is_little_endian, length, encoding)
+                if has_tag_set and tag not in tag_set:
+                    continue
                 yield DataElement(tag, VR, seq, value_tell,
                                   is_undefined_length=True)
             else:
@@ -358,14 +382,19 @@ def data_element_generator(fp,
                     # Store the encoding value in the generator for use
                     # with future elements (SQs)
                     encoding = convert_encodings(encoding)
+                    if not has_specific_char_set:
+                        continue
 
+                # tags with undefined length are skipped after read
+                if has_tag_set and tag not in tag_set:
+                    continue
                 yield RawDataElement(tag, VR, length, value, value_tell,
                                      is_implicit_VR, is_little_endian)
 
 
 def read_dataset(fp, is_implicit_VR, is_little_endian, bytelength=None,
                  stop_when=None, defer_size=None,
-                 parent_encoding=default_encoding):
+                 parent_encoding=default_encoding, specific_tags=None):
     """Return a Dataset instance containing the next dataset in the file.
 
     Parameters
@@ -387,6 +416,8 @@ def read_dataset(fp, is_implicit_VR, is_little_endian, bytelength=None,
     parent_encoding :
         optional encoding to use as a default in case
         a Specific Character Set (0008,0005) isn't specified
+    specific_tags : list or None
+        See ``read_file`` for parameter info.
 
     Returns
     -------
@@ -400,7 +431,8 @@ def read_dataset(fp, is_implicit_VR, is_little_endian, bytelength=None,
     raw_data_elements = dict()
     fpStart = fp.tell()
     de_gen = data_element_generator(fp, is_implicit_VR, is_little_endian,
-                                    stop_when, defer_size, parent_encoding)
+                                    stop_when, defer_size, parent_encoding,
+                                    specific_tags)
     try:
         while (bytelength is None) or (fp.tell() - fpStart < bytelength):
             raw_data_element = next(de_gen)
@@ -635,7 +667,8 @@ def _at_pixel_data(tag, VR, length):
     return tag == (0x7fe0, 0x0010)
 
 
-def read_partial(fileobj, stop_when=None, defer_size=None, force=False):
+def read_partial(fileobj, stop_when=None, defer_size=None,
+                 force=False, specific_tags=None):
     """Parse a DICOM file until a condition is met.
 
     Parameters
@@ -648,6 +681,8 @@ def read_partial(fileobj, stop_when=None, defer_size=None, force=False):
         See ``read_file`` for parameter info.
     force : boolean
         See ``read_file`` for parameter info.
+    specific_tags : list or None
+        See ``read_file`` for parameter info.
 
     Notes
     -----
@@ -741,7 +776,8 @@ def read_partial(fileobj, stop_when=None, defer_size=None, force=False):
     #   the transfer syntax (whether read from the file meta or guessed at)
     try:
         dataset = read_dataset(fileobj, is_implicit_VR, is_little_endian,
-                               stop_when=stop_when, defer_size=defer_size)
+                               stop_when=stop_when, defer_size=defer_size,
+                               specific_tags=specific_tags)
     except EOFError:
         pass  # error already logged in read_dataset
 
@@ -757,7 +793,8 @@ def read_partial(fileobj, stop_when=None, defer_size=None, force=False):
                            is_implicit_VR, is_little_endian)
 
 
-def read_file(fp, defer_size=None, stop_before_pixels=False, force=False):
+def read_file(fp, defer_size=None, stop_before_pixels=False,
+              force=False, specific_tags=None):
     """Read and parse a DICOM dataset stored in the DICOM File Format.
 
     Read a DICOM dataset stored in accordance with the DICOM File Format
@@ -785,6 +822,9 @@ def read_file(fp, defer_size=None, stop_before_pixels=False, force=False):
         If False (default), raises an InvalidDicomError if the file is missing
         the File Meta Information header. Set to True to force reading even if
         no File Meta Information header is found.
+    specific_tags : list or None
+        If not None, only the tags in the list are returned. The list
+        elements can be tags or tag names.
 
     Returns
     -------
@@ -832,8 +872,9 @@ def read_file(fp, defer_size=None, stop_before_pixels=False, force=False):
         logger.debug("\n" + "-" * 80)
         logger.debug("Call to read_file()")
         msg = ("filename:'%s', defer_size='%s', "
-               "stop_before_pixels=%s, force=%s")
-        logger.debug(msg % (fp.name, defer_size, stop_before_pixels, force))
+               "stop_before_pixels=%s, force=%s, specific_tags=%s")
+        logger.debug(msg % (fp.name, defer_size, stop_before_pixels,
+                            force, specific_tags))
         if caller_owns_file:
             logger.debug("Caller passed file object")
         else:
@@ -849,7 +890,7 @@ def read_file(fp, defer_size=None, stop_before_pixels=False, force=False):
         stop_when = _at_pixel_data
     try:
         dataset = read_partial(fp, stop_when, defer_size=defer_size,
-                               force=force)
+                               force=force, specific_tags=specific_tags)
     finally:
         if not caller_owns_file:
             fp.close()
diff --git a/pydicom/sequence.py b/pydicom/sequence.py
index aa7383b85..a484ca4ca 100644
--- a/pydicom/sequence.py
+++ b/pydicom/sequence.py
@@ -60,9 +60,8 @@ class Sequence(MultiValue):
 
     def __repr__(self):
         """String representation of the Sequence."""
-        formatstr = "<%(classname)s, length %(count)d, at %(id)X>"
+        formatstr = "<%(classname)s, length %(count)d>"
         return formatstr % {
             'classname': self.__class__.__name__,
-            'id': id(self),
             'count': len(self)
         }


Test patch:
diff --git a/pydicom/tests/test_charset.py b/pydicom/tests/test_charset.py
index a8b41de2c..e54f516c8 100644
--- a/pydicom/tests/test_charset.py
+++ b/pydicom/tests/test_charset.py
@@ -79,6 +79,17 @@ class charsetTests(unittest.TestCase):
         ds = dicomio.read_file(multiPN_file)
         ds.decode()
 
+    def testEncodingWithSpecificTags(self):
+        """Encoding is correctly applied even if  Specific Character Set
+        is not in specific tags..."""
+        ds = dicomio.read_file(jp_file, specific_tags=['PatientName'])
+        ds.decode()
+        self.assertEqual(1, len(ds))
+        expected = ('Yamada^Tarou='
+                    '\033$B;3ED\033(B^\033$BB@O:\033(B='
+                    '\033$B$d$^$@\033(B^\033$B$?$m$&\033(B')
+        self.assertEqual(expected, ds.PatientName)
+
 
 if __name__ == "__main__":
     # This is called if run alone, but not if loaded through run_tests.py
diff --git a/pydicom/tests/test_dataelem.py b/pydicom/tests/test_dataelem.py
index 392e5704d..2fbc4709d 100644
--- a/pydicom/tests/test_dataelem.py
+++ b/pydicom/tests/test_dataelem.py
@@ -9,6 +9,10 @@
 
 import unittest
 
+import sys
+
+from pydicom.charset import default_encoding
+
 from pydicom.dataelem import DataElement
 from pydicom.dataelem import RawDataElement, DataElement_from_raw
 from pydicom.dataset import Dataset
@@ -201,16 +205,39 @@ class DataElementTests(unittest.TestCase):
 
 
 class RawDataElementTests(unittest.TestCase):
-    def setUp(self):
+    def testKeyError(self):
+        """RawDataElement: conversion of unknown tag throws KeyError..."""
         # raw data element -> tag VR length value
         #                       value_tell is_implicit_VR is_little_endian'
         # Unknown (not in DICOM dict), non-private, non-group 0 for this test
-        self.raw1 = RawDataElement(Tag(0x88880002), None, 4, 0x1111,
-                                   0, True, True)
-
-    def testKeyError(self):
-        """RawDataElement: conversion of unknown tag throws KeyError........"""
-        self.assertRaises(KeyError, DataElement_from_raw, self.raw1)
+        raw = RawDataElement(Tag(0x88880002), None, 4, 0x1111,
+                             0, True, True)
+        self.assertRaises(KeyError, DataElement_from_raw, raw)
+
+    def testValidTag(self):
+        """RawDataElement: conversion of known tag succeeds..."""
+        raw = RawDataElement(Tag(0x00080020), 'DA', 8, b'20170101',
+                             0, False, True)
+        element = DataElement_from_raw(raw, default_encoding)
+        self.assertEqual(element.name, 'Study Date')
+        self.assertEqual(element.VR, 'DA')
+        self.assertEqual(element.value, '20170101')
+
+    @unittest.skipIf(sys.version_info >= (3, ), 'Testing Python 2 behavior')
+    def testTagWithoutEncodingPython2(self):
+        """RawDataElement: no encoding needed in Python 2."""
+        raw = RawDataElement(Tag(0x00104000), 'LT', 23,
+                             b'comment\\comment2\\comment3',
+                             0, False, True)
+        element = DataElement_from_raw(raw)
+        self.assertEqual(element.name, 'Patient Comments')
+
+    @unittest.skipIf(sys.version_info < (3, ), 'Testing Python 3 behavior')
+    def testTagWithoutEncodingPython3(self):
+        """RawDataElement: raises if no encoding given in Python 3."""
+        self.assertRaises(TypeError, RawDataElement(Tag(0x00104000), 'LT', 14,
+                                                    b'comment1\\comment2',
+                                                    0, False, True))
 
 
 if __name__ == "__main__":
diff --git a/pydicom/tests/test_filereader.py b/pydicom/tests/test_filereader.py
index c89d4b15b..5a81c80ae 100644
--- a/pydicom/tests/test_filereader.py
+++ b/pydicom/tests/test_filereader.py
@@ -310,6 +310,43 @@ class ReaderTests(unittest.TestCase):
         missing = [Tag(0x7fe0, 0x10), Tag(0xfffc, 0xfffc)]
         self.assertEqual(ctfull_tags, ctpartial_tags + missing, msg)
 
+    def testSpecificTags(self):
+        """Returns only tags specified by user."""
+        ctspecific = read_file(ct_name, specific_tags=[
+            Tag(0x0010, 0x0010), 'PatientID', 'ImageType', 'ViewName'])
+        ctspecific_tags = sorted(ctspecific.keys())
+        expected = [
+            # ViewName does not exist in the data set
+            Tag(0x0008, 0x0008), Tag(0x0010, 0x0010), Tag(0x0010, 0x0020)
+        ]
+        self.assertEqual(expected, ctspecific_tags)
+
+    def testSpecificTagsWithUnknownLengthSQ(self):
+        """Returns only tags specified by user."""
+        unknown_len_sq_tag = Tag(0x3f03, 0x1001)
+        tags = read_file(priv_SQ_name, specific_tags=[
+            unknown_len_sq_tag])
+        tags = sorted(tags.keys())
+        self.assertEqual([unknown_len_sq_tag], tags)
+
+        tags = read_file(priv_SQ_name, specific_tags=[
+            'PatientName'])
+        tags = sorted(tags.keys())
+        self.assertEqual([], tags)
+
+    def testSpecificTagsWithUnknownLengthTag(self):
+        """Returns only tags specified by user."""
+        unknown_len_tag = Tag(0x7fe0, 0x0010)  # Pixel Data
+        tags = read_file(emri_jpeg_2k_lossless, specific_tags=[
+            unknown_len_tag])
+        tags = sorted(tags.keys())
+        self.assertEqual([unknown_len_tag], tags)
+
+        tags = read_file(emri_jpeg_2k_lossless, specific_tags=[
+            'SpecificCharacterSet'])
+        tags = sorted(tags.keys())
+        self.assertEqual([Tag(0x08, 0x05)], tags)
+
     def testPrivateSQ(self):
         """Can read private undefined length SQ without error."""
         # From issues 91, 97, 98. Bug introduced by fast reading, due to