Repository: pydicom/pydicom

Issue:
Warn when attempting to save unencapsulated compressed datasets
**Is your feature request related to a problem? Please describe.**
Another common gotcha with saving compressed pixel data is that it first needs to be encapsulated.

**Describe the solution you'd like**
I think we should add a simple check when saving that looks to see whether or not Pixel Data has been encapsulated when the transfer syntax is compressed. If not then the user should get a warning (because maybe that was their intention).


Discussion:

Patch:
diff --git a/doc/release_notes/v1.4.0.rst b/doc/release_notes/v1.4.0.rst
index 89e8ed34a..ab11550e6 100644
--- a/doc/release_notes/v1.4.0.rst
+++ b/doc/release_notes/v1.4.0.rst
@@ -69,6 +69,8 @@ Enhancements
   (:issue:`452`)
 * JPEG 2000 (1.2.840.10008.1.2.4.91) transfer syntax is supported for data
   with bit depth > 8 with the Pillow pixel data handler
+* A warning has been added when attempting to save a dataset with compressed
+  transfer syntax but no encapsulation of the pixel data. (:issue:`1007`)
 
 Changes
 .......
diff --git a/pydicom/dataset.py b/pydicom/dataset.py
index c8e8698e0..9bb62bc29 100644
--- a/pydicom/dataset.py
+++ b/pydicom/dataset.py
@@ -15,13 +15,14 @@ Dataset (dict subclass)
             contains its own DataElements, and so on in a recursive manner.
 """
 
-import inspect  # for __dir__
+from bisect import bisect_left
 import io
+import inspect  # for __dir__
+from itertools import takewhile
 import json
 import os
 import os.path
-from bisect import bisect_left
-from itertools import takewhile
+import warnings
 
 import pydicom  # for dcmwrite
 import pydicom.charset
@@ -1758,6 +1759,22 @@ class Dataset(dict):
                 "set appropriately before saving.".format(
                     self.__class__.__name__))
 
+        # If compressed transfer syntax check that Pixel Data is encapsulated
+        try:
+            tsyntax = self.file_meta.TransferSyntaxUID
+            if not tsyntax.is_private and tsyntax.is_compressed:
+                # Check for Basic Offset Table item tag
+                # Must always be little endian - PS3.5 Annex A.4
+                assert self.PixelData[:4] == b'\xFE\xFF\x00\xE0'
+        except AttributeError:
+            pass
+        except AssertionError:
+            warnings.warn(
+                "The dataset uses a compressed Transfer Syntax UID but the "
+                "Pixel Data hasn't been encapsulated. See the "
+                "pydicom.encaps.encapsulate() function for more information"
+            )
+
         pydicom.dcmwrite(filename, self, write_like_original)
 
     def ensure_file_meta(self):


Test patch:
diff --git a/pydicom/tests/test_dataset.py b/pydicom/tests/test_dataset.py
index 19909325b..4513c020d 100644
--- a/pydicom/tests/test_dataset.py
+++ b/pydicom/tests/test_dataset.py
@@ -8,6 +8,7 @@ from pydicom import compat
 from pydicom.data import get_testdata_files
 from pydicom.dataelem import DataElement, RawDataElement
 from pydicom.dataset import Dataset, FileDataset, validate_file_meta
+from pydicom.encaps import encapsulate
 from pydicom import dcmread
 from pydicom.filebase import DicomBytesIO
 from pydicom.overlay_data_handlers import numpy_handler as NP_HANDLER
@@ -17,6 +18,7 @@ from pydicom.tag import Tag
 from pydicom.uid import (
     ImplicitVRLittleEndian,
     ExplicitVRBigEndian,
+    JPEGBaseline,
     PYDICOM_IMPLEMENTATION_UID
 )
 
@@ -1122,6 +1124,67 @@ class TestDataset(object):
         ds.file_meta.ImplementationClassUID = '1.4'
         ds.save_as(fp, write_like_original=False)
 
+    def test_save_as_compressed_no_encaps(self):
+        """Test saving a compressed dataset with no encapsulation."""
+        fp = DicomBytesIO()
+        ds = Dataset()
+        ds.is_little_endian = True
+        ds.is_implicit_VR = False
+        ds.file_meta = Dataset()
+        ds.file_meta.TransferSyntaxUID = JPEGBaseline
+        ds.PixelData = b'\x00\x01\x02\x03\x04\x05\x06'
+        ds['PixelData'].VR = 'OB'
+        msg = (
+            r"The dataset uses a compressed Transfer Syntax UID but the "
+            r"Pixel Data hasn't been encapsulated"
+        )
+        with pytest.warns(UserWarning, match=msg):
+            ds.save_as(fp)
+
+    def test_save_as_compressed_encaps(self):
+        """Test saving a compressed dataset with encapsulation."""
+        fp = DicomBytesIO()
+        ds = Dataset()
+        ds.is_little_endian = True
+        ds.is_implicit_VR = False
+        ds.file_meta = Dataset()
+        ds.file_meta.TransferSyntaxUID = JPEGBaseline
+        ds.PixelData = encapsulate([b'\x00\x01\x02\x03\x04\x05\x06'])
+        ds['PixelData'].VR = 'OB'
+        ds.save_as(fp)
+
+    def test_save_as_no_pixel_data(self):
+        """Test saving with no Pixel Data."""
+        fp = DicomBytesIO()
+        ds = Dataset()
+        ds.is_little_endian = True
+        ds.is_implicit_VR = False
+        ds.file_meta = Dataset()
+        ds.file_meta.TransferSyntaxUID = JPEGBaseline
+        ds.save_as(fp)
+
+    def test_save_as_no_file_meta(self):
+        """Test saving with no Transfer Syntax or file_meta."""
+        fp = DicomBytesIO()
+        ds = Dataset()
+        ds.is_little_endian = True
+        ds.is_implicit_VR = False
+        ds.file_meta = Dataset()
+        ds.save_as(fp)
+
+        del ds.file_meta
+        ds.save_as(fp)
+
+    def test_save_as_private_transfer_syntax(self):
+        """Test saving with a private transfer syntax."""
+        fp = DicomBytesIO()
+        ds = Dataset()
+        ds.is_little_endian = True
+        ds.is_implicit_VR = False
+        ds.file_meta = Dataset()
+        ds.file_meta.TransferSyntaxUID = '1.2.3.4.5.6'
+        ds.save_as(fp)
+
     def test_with(self):
         """Test Dataset.__enter__ and __exit__."""
         test_file = get_testdata_files('CT_small.dcm')[0]