Repository: pydicom/pydicom

Issue:
to_json does not work with binary data in pixel_array
**Describe the issue**
Loading a dicom file and then performing a to_json() on it does not work with binary data in pixel_array.



**Expected behavior**
I would have expected that a base64 conversion is first performed on the binary data and then encoded to json. 

**Steps To Reproduce**
How to reproduce the issue. Please include:
1. A minimum working code sample

import pydicom
ds = pydicom.dcmread('path_to_file')
output = ds.to_json()


2. The traceback (if one occurred)

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/.virtualenvs/my_env/lib/python3.7/site-packages/pydicom/dataset.py", line 2003, in to_json
    dump_handler=dump_handler
  File "/.virtualenvs/my_env/lib/python3.7/site-packages/pydicom/dataset.py", line 1889, in _data_element_to_json
    binary_value = data_element.value.encode('utf-8')
AttributeError: 'bytes' object has no attribute 'encode'


3. Which of the following packages are available and their versions:
  * Numpy
numpy==1.17.2
  * Pillow
Pillow==6.1.0
  * JPEG-LS
  * GDCM
4. The anonymized DICOM dataset (if possible).

**Your environment**
Please run the following and paste the output.
```bash
$ python -c "import platform; print(platform.platform())"
Darwin-19.2.0-x86_64-i386-64bit
$ python -c "import sys; print('Python ', sys.version)"
Python  3.7.6 (default, Dec 30 2019, 19:38:26) 
[Clang 11.0.0 (clang-1100.0.33.16)]
$ python -c "import pydicom; print('pydicom ', pydicom.__version__)"
pydicom  1.3.0
```


Discussion:
pep8speaks: Hello @mrbean-bremen! Thanks for opening this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:





* In the file [`pydicom/tests/test_json.py`](https://github.com/pydicom/pydicom/blob/402fdea001b51196b12f81f5e9de1a7969eb05af/pydicom/tests/test_json.py):

> [Line 355:1](https://github.com/pydicom/pydicom/blob/402fdea001b51196b12f81f5e9de1a7969eb05af/pydicom/tests/test_json.py#L355): [W391](https://duckduckgo.com/?q=pep8%20W391) blank line at end of file


darcymason: > I just didn't add release notes yet to avoid conflicts with the other PR - will add them after that has been merged.

I've merged the other PR so should be good to add the release note changes.

Patch:
diff --git a/doc/release_notes/v1.4.1.rst b/doc/release_notes/v1.4.1.rst
index e0f0e8965..748579f82 100644
--- a/doc/release_notes/v1.4.1.rst
+++ b/doc/release_notes/v1.4.1.rst
@@ -7,3 +7,9 @@ Changelog
 Fixes
 .....
 * Fixed writing of empty sequences (regression, :issue:`1030`)
+
+Changes
+.......
+* In `Dataset.to_json` and `Dataset.to_json_dict`, the default of the
+  `bulk_data_threshold` argument has changed to 1024, and is now ignored if
+  no bulk data handler is set (see :issue:`1029`)
diff --git a/doc/tutorials/dicom_json.rst b/doc/tutorials/dicom_json.rst
new file mode 100644
index 000000000..051c532ce
--- /dev/null
+++ b/doc/tutorials/dicom_json.rst
@@ -0,0 +1,96 @@
+============================
+Introduction to JSON support
+============================
+
+.. versionadded:: 1.3
+
+Starting in *pydicom* version 1.3, some support for converting DICOM data to
+and from JSON format has been added. This support is considered to be in
+beta state, and the API is still subject to change.
+
+Support for the JSON format has been added to the DICOM Standard in
+Part 18 as the :dcm:`DICOM JSON Model<part18/chapter_F.html>`. The standard
+describes how different DICOM value representations can be encoded in JSON.
+
+Converting a dataset into JSON format
+=====================================
+
+*pydicom* supports the conversion of a DICOM dataset both into a JSON string
+and into a deserialized JSON dictionary:
+
+  >>> import pydicom
+  >>> from pydicom.data import get_testdata_file
+  >>> filename = get_testdata_file("CT_small.dcm")
+  >>> ds = pydicom.dcmread(filename)
+  >>> ds.to_json()
+  '{"00080005": {"Value": ["ISO_IR 100"], "vr": "CS"}, "00080008": {"Value":...
+  >>> ds.to_json_dict()
+  {"00080005": {"Value": ["ISO_IR 100"], "vr": "CS"}, "00080008": {"Value":...
+
+Which of these methods you need depends on your use case. The JSON string
+format created by :func:`~pydicom.dataset.Dataset.to_json` can be used in
+low-level APIs to serialize the dataset.
+Higher-level Python APIs (like Django) often work directly with Python
+dictionaries deserialized from a JSON string instead, so
+:func:`~pydicom.dataset.Dataset.to_json_dict` can be more convenient here.
+
+Creating a dataset from JSON
+============================
+
+Similar, a dataset can be created both from a JSON string and from a JSON
+dictionary. There is only a single function to handle both cases:
+
+  >>> from pydicom.dataset import Dataset
+  >>> Dataset.from_json('{"00080005": {"Value": ["ISO_IR 100"], "vr": "CS"}}')
+  (0008, 0005) Specific Character Set              CS: u'ISO_IR 100'
+  >>> Dataset.from_json({"00080005": {"Value": ["ISO_IR 100"], "vr": "CS"}})
+  (0008, 0005) Specific Character Set              CS: u'ISO_IR 100'
+
+The conversion in both directions is symmetric:
+
+  >>> import pydicom
+  >>> filename = pydicom.data.get_testdata_file("CT_small.dcm")
+  >>> ds = pydicom.dcmread(filename)
+  >>> ds_json = ds.to_json()
+  >>> ds1 = pydicom.dataset.Dataset.from_json(ds_json)
+  >>> assert ds == ds1
+
+
+Working with large binary data
+==============================
+
+Large binary data can be handled in two ways. It can be encoded
+:dcm:`inline<part18/sect_F.2.7.html>` as a base64-encoded string, or it can
+be accessed via a :dcm:`BulkDataURI<part18/sect_F.2.6.html>` provided in the
+JSON data, that provides the possibility to retrieve the data using the
+`DICOMweb WADO-RS <https://www.dicomstandard.org/dicomweb/retrieve-wado-rs-and-wado-uri/>`_
+standard.
+
+If you don't provide additional arguments to the encoding functions, the
+data is encoded inline. If you want to save or retrieve data using DICOMweb
+WADO-RS, you have to provide a bulk data handler.
+
+On writing JSON data, the bulk data handler is responsible to store the data
+so it can be retrieved via the ``BulkDataURI`` saved in the JSON dataset.
+Note that only data greater than ``bulk_data_threshold`` (by default set to
+1024) is handled by the bulk data handler - smaller data is encoded inline.
+
+  >>> import pydicom
+  >>> def bulk_data_handler(data_element):
+  >>>     uri = store_data_and_return_uri(data_element)
+  >>>     return uri
+  >>>
+  >>> filename = pydicom.data.get_testdata_file("CT_small.dcm")
+  >>> ds = pydicom.dcmread(filename)
+  >>> ds_json = ds.to_json(bulk_data_element_handler=bulk_data_handler)
+
+On reading JSON data, the handler must be able to retrieve the data using
+the stored ``BulkDataURI``:
+
+  >>> def bulk_data_reader(bulk_data_uri):
+  >>>     return data_retrieved_via_uri(bulk_data_uri)
+  >>>
+  >>> json_data = {
+  >>>     "00091002": {"vr": "OB", "BulkDataURI": "https://my.wado.org/123"}
+  >>> }
+  >>> ds = Dataset.from_json(json_data, bulk_data_uri_handler=bulk_data_reader)
diff --git a/doc/tutorials/index.rst b/doc/tutorials/index.rst
index 73c04d08c..b4723dc63 100644
--- a/doc/tutorials/index.rst
+++ b/doc/tutorials/index.rst
@@ -13,5 +13,6 @@ New to *pydicom*? Then these tutorials should get you up and running.
    virtualenvs
    dataset_basics
    sr_basics
+   dicom_json
    contributing_code
    contributing_docs
diff --git a/pydicom/dataelem.py b/pydicom/dataelem.py
index 71bdec460..007210769 100644
--- a/pydicom/dataelem.py
+++ b/pydicom/dataelem.py
@@ -315,32 +315,21 @@ class DataElement(object):
             via DICOMweb WADO-RS
         bulk_data_threshold: int
             Size of base64 encoded data element above which a value will be
-            provided in form of a "BulkDataURI" rather than "InlineBinary"
+            provided in form of a "BulkDataURI" rather than "InlineBinary".
+            Ignored if no bulk data handler is given.
 
         Returns
         -------
         dict
             Mapping representing a JSON encoded data element
-
-        Raises
-        ------
-        TypeError
-            When size of encoded data element exceeds `bulk_data_threshold`
-            but `bulk_data_element_handler` is ``None`` and hence not callable
-
         """
         json_element = {'vr': self.VR, }
         if self.VR in jsonrep.BINARY_VR_VALUES:
             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:
-                    if bulk_data_element_handler is None:
-                        raise TypeError(
-                            'No bulk data element handler provided to '
-                            'generate URL for value of data element "{}".'
-                            .format(self.name)
-                        )
+                if (bulk_data_element_handler is not None and
+                        len(encoded_value) > bulk_data_threshold):
                     json_element['BulkDataURI'] = bulk_data_element_handler(
                         self
                     )
@@ -398,7 +387,7 @@ class DataElement(object):
             )
         return json_element
 
-    def to_json(self, bulk_data_threshold=1, bulk_data_element_handler=None,
+    def to_json(self, bulk_data_threshold=1024, bulk_data_element_handler=None,
                 dump_handler=None):
         """Return a JSON representation of the :class:`DataElement`.
 
@@ -412,7 +401,8 @@ class DataElement(object):
             via DICOMweb WADO-RS
         bulk_data_threshold: int
             Size of base64 encoded data element above which a value will be
-            provided in form of a "BulkDataURI" rather than "InlineBinary"
+            provided in form of a "BulkDataURI" rather than "InlineBinary".
+            Ignored if no bulk data handler is given.
         dump_handler : callable, optional
             Callable function that accepts a :class:`dict` and returns the
             serialized (dumped) JSON string (by default uses
@@ -423,12 +413,6 @@ class DataElement(object):
         dict
             Mapping representing a JSON encoded data element
 
-        Raises
-        ------
-        TypeError
-            When size of encoded data element exceeds `bulk_data_threshold`
-            but `bulk_data_element_handler` is ``None`` and hence not callable
-
         See also
         --------
         Dataset.to_json
diff --git a/pydicom/dataset.py b/pydicom/dataset.py
index 3a1bb4f6a..1f39b08a7 100644
--- a/pydicom/dataset.py
+++ b/pydicom/dataset.py
@@ -2126,11 +2126,11 @@ class Dataset(dict):
             dataset.add(data_element)
         return dataset
 
-    def to_json_dict(self, bulk_data_threshold=1,
+    def to_json_dict(self, bulk_data_threshold=1024,
                      bulk_data_element_handler=None):
         """Return a dictionary representation of the :class:`Dataset`
         conforming to the DICOM JSON Model as described in the DICOM
-        Standard, Part 18, :dcm:`Annex F<part18/chaptr_F.html>`.
+        Standard, Part 18, :dcm:`Annex F<part18/chapter_F.html>`.
 
         .. versionadded:: 1.4
 
@@ -2140,7 +2140,7 @@ class Dataset(dict):
             Threshold for the length of a base64-encoded binary data element
             above which the element should be considered bulk data and the
             value provided as a URI rather than included inline (default:
-            ``1``).
+            ``1024``). Ignored if no bulk data handler is given.
         bulk_data_element_handler : callable, optional
             Callable function that accepts a bulk data element and returns a
             JSON representation of the data element (dictionary including the
@@ -2161,7 +2161,7 @@ class Dataset(dict):
             )
         return json_dataset
 
-    def to_json(self, bulk_data_threshold=1, bulk_data_element_handler=None,
+    def to_json(self, bulk_data_threshold=1024, bulk_data_element_handler=None,
                 dump_handler=None):
         """Return a JSON representation of the :class:`Dataset`.
 
@@ -2175,7 +2175,7 @@ class Dataset(dict):
             Threshold for the length of a base64-encoded binary data element
             above which the element should be considered bulk data and the
             value provided as a URI rather than included inline (default:
-            ``1``).
+            ``1024``). Ignored if no bulk data handler is given.
         bulk_data_element_handler : callable, optional
             Callable function that accepts a bulk data element and returns a
             JSON representation of the data element (dictionary including the


Test patch:
diff --git a/pydicom/tests/test_json.py b/pydicom/tests/test_json.py
index 2c5ec49eb..6164732d6 100644
--- a/pydicom/tests/test_json.py
+++ b/pydicom/tests/test_json.py
@@ -1,12 +1,11 @@
 # -*- coding: utf-8 -*-
 # Copyright 2008-2019 pydicom authors. See LICENSE file for details.
 import json
-import sys
 
 import pytest
 
 from pydicom import dcmread, compat
-from pydicom.data import get_testdata_files
+from pydicom.data import get_testdata_file
 from pydicom.dataelem import DataElement
 from pydicom.dataset import Dataset
 from pydicom.tag import Tag, BaseTag
@@ -15,7 +14,7 @@ from pydicom.valuerep import PersonNameUnicode, PersonName3
 
 class TestPersonName(object):
     def test_json_pn_from_file(self):
-        with open(get_testdata_files("test_PN.json")[0]) as s:
+        with open(get_testdata_file("test_PN.json")) as s:
             ds = Dataset.from_json(s.read())
         assert isinstance(ds[0x00080090].value,
                           (PersonNameUnicode, PersonName3))
@@ -144,7 +143,7 @@ class TestAT(object):
         assert 0x000910AF == ds[0x00091001].value
         assert [0x00100010, 0x00100020, 0x00100030] == ds[0x00091002].value
 
-    def test_invalid_json(self):
+    def test_invalid_value_in_json(self):
         ds_json = ('{"00091001": {"vr": "AT", "Value": ["000910AG"]}, '
                    '"00091002": {"vr": "AT", "Value": ["00100010"]}}')
         with pytest.warns(UserWarning, match='Invalid value "000910AG" for '
@@ -153,15 +152,24 @@ class TestAT(object):
             assert ds[0x00091001].value is None
             assert 0x00100010 == ds[0x00091002].value
 
+    def test_invalid_tag_in_json(self):
+        ds_json = ('{"000910AG": {"vr": "AT", "Value": ["00091000"]}, '
+                   '"00091002": {"vr": "AT", "Value": ["00100010"]}}')
+        with pytest.raises(ValueError, match='Data element "000910AG" could '
+                                             'not be loaded from JSON:'):
+            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)
+        ds1 = Dataset(dcmread(get_testdata_file("CT_small.dcm")))
+        ds_json = ds1.to_json()
         ds2 = Dataset.from_json(ds_json)
         assert ds1 == ds2
 
-        ds_json = ds1.to_json_dict(bulk_data_threshold=100000)
+        ds_json = ds1.to_json_dict()
         ds2 = Dataset.from_json(ds_json)
         assert ds1 == ds2
 
@@ -205,7 +213,7 @@ class TestDataSetToJson(object):
         ds.add_new(0x00091101, 'SH', 'Version2')
         ds.add_new(0x00091102, 'US', 2)
 
-        json_string = ds.to_json(bulk_data_threshold=100)
+        json_string = ds.to_json()
         json_model = json.loads(json_string)
 
         assert json_model['00080005']['Value'] == ['ISO_IR 100']
@@ -223,7 +231,7 @@ class TestDataSetToJson(object):
         ds2 = Dataset.from_json(json_model)
         assert ds == ds2
 
-        json_model2 = ds.to_json_dict(bulk_data_threshold=100)
+        json_model2 = ds.to_json_dict()
         if compat.in_py2:
             # in Python 2, the encoding of this is slightly different
             # (single vs double quotation marks)
@@ -274,7 +282,7 @@ class TestDataSetToJson(object):
 
 class TestSequence(object):
     def test_nested_sequences(self):
-        test1_json = get_testdata_files("test1.json")[0]
+        test1_json = get_testdata_file("test1.json")
         with open(test1_json) as f:
             with pytest.warns(UserWarning,
                               match='no bulk data URI handler provided '):
@@ -288,7 +296,7 @@ class TestBinary(object):
     def test_inline_binary(self):
         ds = Dataset()
         ds.add_new(0x00091002, 'OB', b'BinaryContent')
-        ds_json = ds.to_json_dict(bulk_data_threshold=20)
+        ds_json = ds.to_json_dict()
         assert "00091002" in ds_json
         assert "QmluYXJ5Q29udGVudA==" == ds_json["00091002"]["InlineBinary"]
         ds1 = Dataset.from_json(ds_json)