Repository: pydicom/pydicom

Issue:
To_Json 'str' object has no attribute 'components'
<!-- Instructions For Filing a Bug: https://github.com/pydicom/pydicom/blob/master/CONTRIBUTING.md#filing-bugs -->

#### Description
<!-- Example: Attribute Error thrown when printing (0x0010, 0x0020) patient Id> 0-->

When converting a dataset to json the following error occurs.
```
Traceback (most recent call last):
  File "/anaconda3/lib/python3.6/threading.py", line 916, in _bootstrap_inner
    self.run()
  File "/anaconda3/lib/python3.6/threading.py", line 864, in run
    self._target(*self._args, **self._kwargs)
  File "~/pacs-proxy/pacs/service.py", line 172, in saveFunction
    jsonObj = ds.to_json()
  File "~/lib/python3.6/site-packages/pydicom/dataset.py", line 2046, in to_json
    dump_handler=dump_handler
  File "~/lib/python3.6/site-packages/pydicom/dataelem.py", line 447, in to_json
    if len(elem_value.components) > 2:
AttributeError: 'str' object has no attribute 'components'
```
#### Steps/Code to Reproduce

ds = pydicom.dcmread("testImg")
jsonObj = ds.to_json()

I'm working on getting an anonymous version of the image, will update. But any advice, suggestions would be appreciated.

#### 

Discussion:
mrbean-bremen: Also, after checking the DICOM standard found that AT is not correctly encoded. Back to WIP.
mrbean-bremen: I'm done here (this fixes the bug and a couple of other things), but I may return here later - there are still some things to do (like testing the binary encodings, fixing that strange sequence problem and refactoring the code a bit).

Patch:
diff --git a/doc/release_notes/v1.4.0.rst b/doc/release_notes/v1.4.0.rst
index 844c43ee0..44fd87977 100644
--- a/doc/release_notes/v1.4.0.rst
+++ b/doc/release_notes/v1.4.0.rst
@@ -6,6 +6,7 @@ Changelog
 
 Fixes
 .....
+* Fixed handling of VRs AT and PN in json encoding (:issue:`915`)
 * Fixed handling of binary values in json encoding (:issue:`887`)
 * Prevent exception if assigning `None` to UI element (:issue:`894`)
 * Fixed print output for numeric multi-value elements (:issue:`892`)
diff --git a/examples/dicomtree.py b/examples/dicomtree.py
index 2cd45263d..ce3c3d5ba 100644
--- a/examples/dicomtree.py
+++ b/examples/dicomtree.py
@@ -1,15 +1,15 @@
+# Copyright pydicom authors 2019. See LICENSE file for details
 """
+sudo apt install tix-dev
+sudo pip install -U pydicom
+python3 dicomtree.py file.dcm
+
 =========================================
 Show a dicom file using hierarchical tree
 =========================================
-
 Show a dicom file using a hierarchical tree in a graphical window.
-
 """
 
-# authors : Guillaume Lemaitre <g.lemaitre58@gmail.com>
-# license : MIT
-
 from __future__ import print_function
 
 from pydicom import compat
@@ -38,6 +38,13 @@ def RunTree(w, filename):
     box.add('exit', text='Exit', underline=0, command=w.destroy, width=6)
     box.pack(side=tkinter_tix.BOTTOM, fill=tkinter_tix.X)
     top.pack(side=tkinter_tix.TOP, fill=tkinter_tix.BOTH, expand=1)
+    # https://stackoverflow.com/questions/17355902/python-tkinter-binding-mousewheel-to-scrollbar
+    tree.bind_all('<MouseWheel>', lambda event:  # Wheel in Windows
+                  tree.hlist.yview_scroll(int(-1*event.delta/120.), "units"))
+    tree.bind_all('<Button-4>', lambda event:  # Wheel up in Linux
+                  tree.hlist.yview_scroll(int(-1), "units"))
+    tree.bind_all('<Button-5>', lambda event:  # Wheel down in Linux
+                  tree.hlist.yview_scroll(int(+1), "units"))
 
     show_file(filename, tree)
 
@@ -79,7 +86,7 @@ if __name__ == '__main__':
         print(usage)
         sys.exit(-1)
     root = tkinter_tix.Tk()
-    root.geometry("{0:d}x{1:d}+{2:d}+{3:d}".format(800, 600, 0, 0))
+    root.geometry("{0:d}x{1:d}+{2:d}+{3:d}".format(1200, 900, 0, 0))
 
     RunTree(root, sys.argv[1])
     root.mainloop()
diff --git a/pydicom/dataelem.py b/pydicom/dataelem.py
index 916a85924..7f23c35de 100644
--- a/pydicom/dataelem.py
+++ b/pydicom/dataelem.py
@@ -329,41 +329,60 @@ class DataElement(object):
                     # Some DICOMweb services get this wrong, so we
                     # workaround the issue and warn the user
                     # rather than raising an error.
-                    logger.error(
+                    logger.warning(
                         'value of data element "{}" with VR Person Name (PN) '
                         'is not formatted correctly'.format(tag)
                     )
                     elem_value.append(v)
                 else:
-                    elem_value.extend(list(v.values()))
-            if vm == '1':
+                    if 'Phonetic' in v:
+                        comps = ['', '', '']
+                    elif 'Ideographic' in v:
+                        comps = ['', '']
+                    else:
+                        comps = ['']
+                    if 'Alphabetic' in v:
+                        comps[0] = v['Alphabetic']
+                    if 'Ideographic' in v:
+                        comps[1] = v['Ideographic']
+                    if 'Phonetic' in v:
+                        comps[2] = v['Phonetic']
+                    elem_value.append('='.join(comps))
+            if len(elem_value) == 1:
+                elem_value = elem_value[0]
+            elif not elem_value:
+                elem_value = empty_value_for_VR(vr)
+        elif vr == 'AT':
+            elem_value = []
+            for v in value:
                 try:
-                    elem_value = elem_value[0]
-                except IndexError:
-                    elem_value = ''
+                    elem_value.append(int(v, 16))
+                except ValueError:
+                    warnings.warn('Invalid value "{}" for AT element - '
+                                  'ignoring it'.format(v))
+                value = value[0]
+            if not elem_value:
+                elem_value = empty_value_for_VR(vr)
+            elif len(elem_value) == 1:
+                elem_value = elem_value[0]
         else:
-            if vm == '1':
-                if value_key == 'InlineBinary':
-                    elem_value = base64.b64decode(value)
-                elif value_key == 'BulkDataURI':
-                    if bulk_data_uri_handler is None:
-                        logger.warning(
-                            'no bulk data URI handler provided for retrieval '
-                            'of value of data element "{}"'.format(tag)
-                        )
-                        elem_value = b''
-                    else:
-                        elem_value = bulk_data_uri_handler(value)
+            if isinstance(value, list) and len(value) == 1:
+                value = value[0]
+            if value_key == 'InlineBinary':
+                elem_value = base64.b64decode(value)
+            elif value_key == 'BulkDataURI':
+                if bulk_data_uri_handler is None:
+                    logger.warning(
+                        'no bulk data URI handler provided for retrieval '
+                        'of value of data element "{}"'.format(tag)
+                    )
+                    elem_value = empty_value_for_VR(vr, raw=True)
                 else:
-                    if value:
-                        elem_value = value[0]
-                    else:
-                        elem_value = value
+                    elem_value = bulk_data_uri_handler(value)
             else:
                 elem_value = value
         if elem_value is None:
-            logger.warning('missing value for data element "{}"'.format(tag))
-            elem_value = ''
+            elem_value = empty_value_for_VR(vr)
 
         elem_value = jsonrep.convert_to_python_number(elem_value, vr)
 
@@ -404,11 +423,9 @@ class DataElement(object):
             but `bulk_data_element_handler` is ``None`` and hence not callable
 
         """
-        # TODO: Determine whether more VRs need to be converted to strings
-        _VRs_TO_QUOTE = ['AT', ]
         json_element = {'vr': self.VR, }
         if self.VR in jsonrep.BINARY_VR_VALUES:
-            if self.value is not None:
+            if not self.is_empty:
                 binary_value = self.value
                 encoded_value = base64.b64encode(binary_value).decode('utf-8')
                 if len(encoded_value) > bulk_data_threshold:
@@ -440,35 +457,35 @@ class DataElement(object):
             ]
             json_element['Value'] = value
         elif self.VR == 'PN':
-            elem_value = self.value
-            if elem_value is not None:
-                if compat.in_py2:
-                    elem_value = PersonNameUnicode(elem_value, 'UTF8')
-                if len(elem_value.components) > 2:
-                    json_element['Value'] = [
-                        {'Phonetic': elem_value.components[2], },
-                    ]
-                elif len(elem_value.components) > 1:
-                    json_element['Value'] = [
-                        {'Ideographic': elem_value.components[1], },
-                    ]
+            if not self.is_empty:
+                elem_value = []
+                if self.VM > 1:
+                    value = self.value
                 else:
-                    json_element['Value'] = [
-                        {'Alphabetic': elem_value.components[0], },
-                    ]
+                    value = [self.value]
+                for v in value:
+                    if compat.in_py2:
+                        v = PersonNameUnicode(v, 'UTF8')
+                    comps = {'Alphabetic': v.components[0]}
+                    if len(v.components) > 1:
+                        comps['Ideographic'] = v.components[1]
+                    if len(v.components) > 2:
+                        comps['Phonetic'] = v.components[2]
+                    elem_value.append(comps)
+                json_element['Value'] = elem_value
+        elif self.VR == 'AT':
+            if not self.is_empty:
+                value = self.value
+                if self.VM == 1:
+                    value = [value]
+                json_element['Value'] = [format(v, '08X') for v in value]
         else:
-            if self.value is not None:
-                is_multivalue = isinstance(self.value, MultiValue)
-                if self.VM > 1 or is_multivalue:
+            if not self.is_empty:
+                if self.VM > 1:
                     value = self.value
                 else:
                     value = [self.value]
-                # ensure it's a list and not another iterable
-                # (e.g. tuple), which would not be JSON serializable
-                if self.VR in _VRs_TO_QUOTE:
-                    json_element['Value'] = [str(v) for v in value]
-                else:
-                    json_element['Value'] = [v for v in value]
+                json_element['Value'] = [v for v in value]
         if hasattr(json_element, 'Value'):
             json_element['Value'] = jsonrep.convert_to_python_number(
                 json_element['Value'], self.VR
diff --git a/pydicom/jsonrep.py b/pydicom/jsonrep.py
index f6b12635e..d96e79cfa 100644
--- a/pydicom/jsonrep.py
+++ b/pydicom/jsonrep.py
@@ -2,6 +2,8 @@
 """Methods for converting Datasets and DataElements to/from json"""
 
 # Order of keys is significant!
+from pydicom.compat import int_type
+
 JSON_VALUE_KEYS = ('Value', 'BulkDataURI', 'InlineBinary',)
 
 BINARY_VR_VALUES = ['OW', 'OB', 'OD', 'OF', 'OL', 'UN',
@@ -30,7 +32,7 @@ def convert_to_python_number(value, vr):
         return None
     number_type = None
     if vr in VRs_TO_BE_INTS:
-        number_type = int
+        number_type = int_type
     if vr in VRs_TO_BE_FLOATS:
         number_type = float
     if number_type is not None:
diff --git a/pydicom/valuerep.py b/pydicom/valuerep.py
index 00d483f34..38e36fe30 100644
--- a/pydicom/valuerep.py
+++ b/pydicom/valuerep.py
@@ -646,9 +646,13 @@ class PersonName3(object):
             # this is the decoded string - save the original string if
             # available for easier writing back
             self.original_string = original_string
-            self._components = tuple(val.split('='))
+            components = val.split('=')
+            # Remove empty elements from the end to avoid trailing '='
+            while len(components) and not components[-1]:
+                components.pop()
+            self._components = tuple(components)
 
-        # if the encoding is not given, leave it as undefined (None)
+            # if the encoding is not given, leave it as undefined (None)
         self.encodings = _verify_encodings(encodings)
         self._dict = {}
 
@@ -812,7 +816,7 @@ class PersonName3(object):
 
     def __bool__(self):
         if self.original_string is None:
-            return (self._components is not None and
+            return (bool(self._components) and
                     (len(self._components) > 1 or bool(self._components[0])))
         return bool(self.original_string)
 


Test patch:
diff --git a/pydicom/tests/test_json.py b/pydicom/tests/test_json.py
index ae4e6c78f..3a078d6db 100644
--- a/pydicom/tests/test_json.py
+++ b/pydicom/tests/test_json.py
@@ -1,89 +1,226 @@
 # -*- coding: utf-8 -*-
 # Copyright 2008-2019 pydicom authors. See LICENSE file for details.
+import json
+
 import pytest
+
+from pydicom import dcmread, compat
 from pydicom.data import get_testdata_files
 from pydicom.dataelem import DataElement
 from pydicom.dataset import Dataset
-from pydicom import compat
-
+from pydicom.tag import Tag, BaseTag
 from pydicom.valuerep import PersonNameUnicode, PersonName3
 
 
-def test_json_PN():
-    s = open(get_testdata_files("test_PN.json")[0], "r").read()
-    ds = Dataset.from_json(s)
-    assert isinstance(ds[0x00080090].value,
-                      (PersonNameUnicode, PersonName3))
-    assert isinstance(ds[0x00100010].value,
-                      (PersonNameUnicode, PersonName3))
-    inner_seq = ds[0x04000561].value[0][0x04000550]
-    dataelem = inner_seq[0][0x00100010]
-    assert isinstance(dataelem.value, (PersonNameUnicode, PersonName3))
-
-
-@pytest.mark.skipif(compat.in_py2,
-                    reason='JSON conversion not yet working in Python 2')
-def test_dataelem_from_json():
-    tag = 0x0080090
-    vr = "PN"
-    value = [{"Alphabetic": ""}]
-    dataelem = DataElement.from_json(Dataset, tag, vr, value, "Value")
-    assert isinstance(dataelem.value, (PersonNameUnicode, PersonName3))
-
-
-def test_json_roundtrip():
-    ds = Dataset()
-    ds.add_new(0x00080005, 'CS', 'ISO_IR 100')
-    ds.add_new(0x00090010, 'LO', 'Creator 1.0')
-    ds.add_new(0x00091001, 'SH', 'Version1')
-    ds.add_new(0x00091002, 'OB', b'BinaryContent')
-    ds.add_new(0x00091003, 'OW', b'\x0102\x3040\x5060')
-    ds.add_new(0x00091004, 'OF', b'\x00\x01\x02\x03\x04\x05\x06\x07')
-    ds.add_new(0x00091005, 'OD', b'\x00\x01\x02\x03\x04\x05\x06\x07'
-                                 b'\x01\x01\x02\x03\x04\x05\x06\x07')
-    ds.add_new(0x00091006, 'OL', b'\x00\x01\x02\x03\x04\x05\x06\x07'
-                                 b'\x01\x01\x02\x03')
-    ds.add_new(0x00091007, 'UI', '1.2.3.4.5.6')
-    ds.add_new(0x00091008, 'DA', '20200101')
-    ds.add_new(0x00091009, 'TM', '115500')
-    ds.add_new(0x0009100a, 'DT', '20200101115500.000000')
-    ds.add_new(0x0009100b, 'UL', 3000000000)
-    ds.add_new(0x0009100c, 'SL', -2000000000)
-    ds.add_new(0x0009100d, 'US', 40000)
-    ds.add_new(0x0009100e, 'SS', -22222)
-    ds.add_new(0x0009100f, 'FL', 3.14)
-    ds.add_new(0x00091010, 'FD', 3.14159265)
-    ds.add_new(0x00091011, 'CS', 'TEST MODE')
-    ds.add_new(0x00091012, 'PN', 'CITIZEN^1')
-    ds.add_new(0x00091013, 'PN', u'Yamada^Tarou=山田^太郎=やまだ^たろう')
-    ds.add_new(0x00091014, 'IS', '42')
-    ds.add_new(0x00091015, 'DS', '3.14159265')
-    ds.add_new(0x00091016, 'AE', b'CONQUESTSRV1')
-    ds.add_new(0x00091017, 'AS', '055Y')
-    ds.add_new(0x00091018, 'LT', 50 * u'Калинка,')
-    ds.add_new(0x00091019, 'UC', 'LONG CODE VALUE')
-    ds.add_new(0x0009101a, 'UN', b'\x0102\x3040\x5060')
-    ds.add_new(0x0009101b, 'UR', 'https://example.com')
-    ds.add_new(0x0009101c, 'AT', b'\x10\x00\x20\x00\x10\x00\x30\x00\x10')
-    ds.add_new(0x0009101d, 'ST', 100 * u'علي بابا')
-    ds.add_new(0x0009101e, 'SH', u'Διονυσιος')
-    ds.add_new(0x00090011, 'LO', 'Creator 2.0')
-    ds.add_new(0x00091101, 'SH', 'Version2')
-    ds.add_new(0x00091102, 'US', 2)
-
-    jsonmodel = ds.to_json(bulk_data_threshold=100)
-    ds2 = Dataset.from_json(jsonmodel)
-
-    assert ds2.SpecificCharacterSet == ['ISO_IR 100']
-
-
-def test_json_private_DS_VM():
-    test1_json = get_testdata_files("test1.json")[0]
-    jsonmodel = open(test1_json, 'r').read()
-    ds = Dataset.from_json(jsonmodel)
-    import json
-    jsonmodel2 = ds.to_json(dump_handler=lambda d: json.dumps(d, indent=2))
-    ds2 = Dataset.from_json(jsonmodel2)
-
-    assert ds.PatientIdentityRemoved == 'YES'
-    assert ds2.PatientIdentityRemoved == 'YES'
+class TestPersonName(object):
+    def test_json_PN_from_file(self):
+        with open(get_testdata_files("test_PN.json")[0]) as s:
+            ds = Dataset.from_json(s.read())
+        assert isinstance(ds[0x00080090].value,
+                          (PersonNameUnicode, PersonName3))
+        assert isinstance(ds[0x00100010].value,
+                          (PersonNameUnicode, PersonName3))
+        inner_seq = ds[0x04000561].value[0][0x04000550]
+        dataelem = inner_seq[0][0x00100010]
+        assert isinstance(dataelem.value, (PersonNameUnicode, PersonName3))
+
+    def test_PN_components_to_json(self):
+        def check_name(tag, components):
+            # we cannot directly compare the dictionaries, as they are not
+            # ordered in Python 2
+            value = ds_json[tag]['Value']
+            assert 1 == len(value)
+            value = value[0]
+            if len(components) == 3:
+                assert components[2] == value['Phonetic']
+            else:
+                assert 'Phonetic' not in value
+            if len(components) >= 2:
+                assert components[1] == value['Ideographic']
+            else:
+                assert 'Ideographic' not in value
+            assert components[0] == value['Alphabetic']
+
+        ds = Dataset()
+        ds.add_new(0x00100010, 'PN', u'Yamada^Tarou=山田^太郎=やまだ^たろう')
+        ds.add_new(0x00091001, 'PN', u'Yamada^Tarou')
+        ds.add_new(0x00091002, 'PN', u'Yamada^Tarou==')
+        ds.add_new(0x00091003, 'PN', u'=山田^太郎=やまだ^たろう')
+        ds.add_new(0x00091004, 'PN', u'Yamada^Tarou==やまだ^たろう')
+        ds.add_new(0x00091005, 'PN', u'==やまだ^たろう')
+        ds.add_new(0x00091006, 'PN', u'=山田^太郎')
+        ds.add_new(0x00091007, 'PN', u'Yamada^Tarou=山田^太郎')
+        ds_json = json.loads(ds.to_json())
+        check_name('00100010', ['Yamada^Tarou', u'山田^太郎', u'やまだ^たろう'])
+        check_name('00091001', ['Yamada^Tarou'])
+        check_name('00091002', ['Yamada^Tarou'])
+        check_name('00091003', ['', u'山田^太郎', u'やまだ^たろう'])
+        check_name('00091004', ['Yamada^Tarou', '', u'やまだ^たろう'])
+        check_name('00091005', ['', '', u'やまだ^たろう'])
+        check_name('00091006', ['', u'山田^太郎'])
+        check_name('00091007', ['Yamada^Tarou', u'山田^太郎'])
+
+    def test_PN_components_from_json(self):
+        # this is the encoded dataset from the previous test, with some
+        # empty components omitted
+        ds_json = (u'{"00100010": {"vr": "PN", "Value": [{"Alphabetic": '
+                   u'"Yamada^Tarou", "Ideographic": "山田^太郎", '
+                   u'"Phonetic": "やまだ^たろう"}]}, '
+                   u'"00091001": {"vr": "PN", "Value": '
+                   u'[{"Alphabetic": "Yamada^Tarou"}]}, '
+                   u'"00091002": {"vr": "PN", "Value": '
+                   u'[{"Alphabetic": "Yamada^Tarou", "Ideographic": "", '
+                   u'"Phonetic": ""}]}, '
+                   u'"00091003": {"vr": "PN", "Value": [{'
+                   u'"Ideographic": "山田^太郎", '
+                   u'"Phonetic": "やまだ^たろう"}]}, '
+                   u'"00091004": {"vr": "PN", "Value": '
+                   u'[{"Alphabetic": "Yamada^Tarou", '
+                   u'"Phonetic": "やまだ^たろう"}]}, '
+                   u'"00091005": {"vr": "PN", "Value": '
+                   u'[{"Phonetic": "やまだ^たろう"}]}, '
+                   u'"00091006": {"vr": "PN", "Value":'
+                   u' [{"Ideographic": "山田^太郎"}]}, '
+                   u'"00091007": {"vr": "PN", "Value": '
+                   u'[{"Alphabetic": "Yamada^Tarou", '
+                   u'"Ideographic": "山田^太郎"}]}}')
+        if compat.in_py2:
+            ds_json = ds_json.encode('UTF8')
+
+        ds = Dataset.from_json(ds_json)
+        assert u'Yamada^Tarou=山田^太郎=やまだ^たろう' == ds.PatientName
+        assert u'Yamada^Tarou' == ds[0x00091001].value
+        assert u'Yamada^Tarou' == ds[0x00091002].value
+        assert u'=山田^太郎=やまだ^たろう' == ds[0x00091003].value
+        assert u'Yamada^Tarou==やまだ^たろう' == ds[0x00091004].value
+        assert u'==やまだ^たろう' == ds[0x00091005].value
+        assert u'=山田^太郎' == ds[0x00091006].value
+        assert u'Yamada^Tarou=山田^太郎' == ds[0x00091007].value
+
+    def test_empty_value(self):
+        ds = Dataset()
+        ds.add_new(0x00100010, 'PN', '')
+        ds_json = json.loads(ds.to_json())
+        assert '00100010' in ds_json
+        assert 'Value' not in ds_json['00100010']
+
+    def test_multi_value_to_json(self):
+        ds = Dataset()
+        patient_names = [u'Buc^Jérôme', u'Διονυσιος', u'Люкceмбypг']
+        ds.add_new(0x00091001, 'PN', patient_names)
+        ds_json = json.loads(ds.to_json())
+        assert [{'Alphabetic': u'Buc^Jérôme'},
+                {'Alphabetic': u'Διονυσιος'},
+                {'Alphabetic': u'Люкceмбypг'}] == ds_json['00091001']['Value']
+
+    def test_dataelem_from_json(self):
+        tag = 0x0080090
+        vr = "PN"
+        value = [{"Alphabetic": ""}]
+        dataelem = DataElement.from_json(Dataset, tag, vr, value, "Value")
+        assert isinstance(dataelem.value, (PersonNameUnicode, PersonName3))
+
+
+class TestAT(object):
+    def test_to_json(self):
+        ds = Dataset()
+        ds.add_new(0x00091001, 'AT', [0x00100010, 0x00100020])
+        ds.add_new(0x00091002, 'AT', Tag(0x28, 0x02))
+        ds.add_new(0x00091003, 'AT', BaseTag(0x00280002))
+        ds.add_new(0x00091004, 'AT', [0x00280002, Tag('PatientName')])
+        ds_json = json.loads(ds.to_json())
+
+        assert ['00100010', '00100020'] == ds_json['00091001']['Value']
+        assert ['00280002'] == ds_json['00091002']['Value']
+        assert ['00280002'] == ds_json['00091003']['Value']
+        assert ['00280002', '00100010'] == ds_json['00091004']['Value']
+
+    def test_from_json(self):
+        ds_json = ('{"00091001": {"vr": "AT", "Value": ["000910AF"]}, '
+                   '"00091002": {"vr": "AT", "Value": ["00100010", '
+                   '"00100020", "00100030"]}}')
+        ds = Dataset.from_json(ds_json)
+        assert 0x000910AF == ds[0x00091001].value
+        assert [0x00100010, 0x00100020, 0x00100030] == ds[0x00091002].value
+
+    def test_invalid_json(self):
+        ds_json = ('{"00091001": {"vr": "AT", "Value": ["000910AG"]}, '
+                   '"00091002": {"vr": "AT", "Value": ["00100010"]}}')
+        with pytest.warns(UserWarning, match='Invalid value "000910AG" for '
+                                             'AT element - ignoring it'):
+            ds = Dataset.from_json(ds_json)
+            assert ds[0x00091001].value is None
+            assert 0x00100010 == ds[0x00091002].value
+
+
+class TestDataSetToJson(object):
+    def test_json_from_dicom_file(self):
+        ds1 = Dataset(dcmread(get_testdata_files("CT_small.dcm")[0]))
+        ds_json = ds1.to_json(bulk_data_threshold=100000)
+        ds2 = Dataset.from_json(ds_json)
+        assert ds1 == ds2
+
+    def test_roundtrip(self):
+        ds = Dataset()
+        ds.add_new(0x00080005, 'CS', 'ISO_IR 100')
+        ds.add_new(0x00090010, 'LO', 'Creator 1.0')
+        ds.add_new(0x00091001, 'SH', 'Version1')
+        ds.add_new(0x00091002, 'OB', b'BinaryContent')
+        ds.add_new(0x00091003, 'OW', b'\x0102\x3040\x5060')
+        ds.add_new(0x00091004, 'OF', b'\x00\x01\x02\x03\x04\x05\x06\x07')
+        ds.add_new(0x00091005, 'OD', b'\x00\x01\x02\x03\x04\x05\x06\x07'
+                                     b'\x01\x01\x02\x03\x04\x05\x06\x07')
+        ds.add_new(0x00091006, 'OL', b'\x00\x01\x02\x03\x04\x05\x06\x07'
+                                     b'\x01\x01\x02\x03')
+        ds.add_new(0x00091007, 'UI', '1.2.3.4.5.6')
+        ds.add_new(0x00091008, 'DA', '20200101')
+        ds.add_new(0x00091009, 'TM', '115500')
+        ds.add_new(0x0009100a, 'DT', '20200101115500.000000')
+        ds.add_new(0x0009100b, 'UL', 3000000000)
+        ds.add_new(0x0009100c, 'SL', -2000000000)
+        ds.add_new(0x0009100d, 'US', 40000)
+        ds.add_new(0x0009100e, 'SS', -22222)
+        ds.add_new(0x0009100f, 'FL', 3.14)
+        ds.add_new(0x00091010, 'FD', 3.14159265)
+        ds.add_new(0x00091011, 'CS', 'TEST MODE')
+        ds.add_new(0x00091012, 'PN', 'CITIZEN^1')
+        ds.add_new(0x00091013, 'PN', u'Yamada^Tarou=山田^太郎=やまだ^たろう')
+        ds.add_new(0x00091014, 'IS', '42')
+        ds.add_new(0x00091015, 'DS', '3.14159265')
+        ds.add_new(0x00091016, 'AE', b'CONQUESTSRV1')
+        ds.add_new(0x00091017, 'AS', '055Y')
+        ds.add_new(0x00091018, 'LT', 50 * u'Калинка,')
+        ds.add_new(0x00091019, 'UC', 'LONG CODE VALUE')
+        ds.add_new(0x0009101a, 'UN', b'\x0102\x3040\x5060')
+        ds.add_new(0x0009101b, 'UR', 'https://example.com')
+        ds.add_new(0x0009101c, 'AT', [0x00100010, 0x00100020])
+        ds.add_new(0x0009101d, 'ST', 100 * u'علي بابا')
+        ds.add_new(0x0009101e, 'SH', u'Διονυσιος')
+        ds.add_new(0x00090011, 'LO', 'Creator 2.0')
+        ds.add_new(0x00091101, 'SH', 'Version2')
+        ds.add_new(0x00091102, 'US', 2)
+
+        json_string = ds.to_json(bulk_data_threshold=100)
+        json_model = json.loads(json_string)
+        assert json_model['00080005']['Value'] == ['ISO_IR 100']
+        assert json_model['00091007']['Value'] == ['1.2.3.4.5.6']
+        assert json_model['0009100A']['Value'] == ['20200101115500.000000']
+        assert json_model['0009100B']['Value'] == [3000000000]
+        assert json_model['0009100C']['Value'] == [-2000000000]
+        assert json_model['0009100D']['Value'] == [40000]
+        assert json_model['0009100F']['Value'] == [3.14]
+        assert json_model['00091010']['Value'] == [3.14159265]
+        assert json_model['00091018']['Value'] == [50 * u'Калинка,']
+
+        ds2 = Dataset.from_json(json_string)
+        assert ds == ds2
+
+    def test_json_private_DS_VM(self):
+        test1_json = get_testdata_files("test1.json")[0]
+        jsonmodel = open(test1_json, 'r').read()
+        ds = Dataset.from_json(jsonmodel)
+        jsonmodel2 = ds.to_json(dump_handler=lambda d: json.dumps(d, indent=2))
+        ds2 = Dataset.from_json(jsonmodel2)
+
+        assert ds.PatientIdentityRemoved == 'YES'
+        assert ds2.PatientIdentityRemoved == 'YES'