Repository: pydicom/pydicom

Issue:
Question on compression of Implicit VR datasets
Using pydicom 2.2.2 and using the example at https://pydicom.github.io/pydicom/stable/tutorials/pixel_data/compressing.html#compressing-using-pydicom

If we read a dataset that is using the Implicit VR Endian transfer syntax
compress it using RLE Lossless 
and write it out to a new file
then the new dataset is written with Implicit VR 
but my expectation is that RLE Lossless should use Explicit VR

setting is_implicit_VR = False after calling compress appears to resolve the issue for me.

My question is:
should compress set is_implicit_VR as all compressed transfer syntaxes should be explicit as far as I am aware, 
or could the documentation be updated to highlight the need to set is_implicit_VR, 
or should I already be aware of this and I've missed something somewhere?



Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/pydicom/pydicom/pull/1566?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom) Report
> Merging [#1566](https://codecov.io/gh/pydicom/pydicom/pull/1566?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom) (670e928) into [master](https://codecov.io/gh/pydicom/pydicom/commit/d5f9ae9223f4c65a479967e76d6ae5abecefd345?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom) (d5f9ae9) will **not change** coverage.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/pydicom/pydicom/pull/1566/graphs/tree.svg?width=650&height=150&src=pr&token=YciLV15Lwr&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom)](https://codecov.io/gh/pydicom/pydicom/pull/1566?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom)

```diff
@@           Coverage Diff           @@
##           master    #1566   +/-   ##
=======================================
  Coverage   97.52%   97.52%           
=======================================
  Files          66       66           
  Lines       10396    10396           
=======================================
  Hits        10139    10139           
  Misses        257      257           
```


| [Impacted Files](https://codecov.io/gh/pydicom/pydicom/pull/1566?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom) | Coverage Δ | |
|---|---|---|
| [pydicom/dataset.py](https://codecov.io/gh/pydicom/pydicom/pull/1566/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom#diff-cHlkaWNvbS9kYXRhc2V0LnB5) | `99.06% <100.00%> (+<0.01%)` | :arrow_up: |
| [pydicom/filewriter.py](https://codecov.io/gh/pydicom/pydicom/pull/1566/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom#diff-cHlkaWNvbS9maWxld3JpdGVyLnB5) | `97.82% <100.00%> (-0.02%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pydicom/pydicom/pull/1566?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pydicom/pydicom/pull/1566?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom). Last update [d5f9ae9...670e928](https://codecov.io/gh/pydicom/pydicom/pull/1566?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom).

pep8speaks: Hello @scaramallion! Thanks for updating 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/filewriter.py`](https://github.com/pydicom/pydicom/blob/2734f200ad475bc2e67e37512977a71858eda3ba/pydicom/filewriter.py):

> [Line 1049:5](https://github.com/pydicom/pydicom/blob/2734f200ad475bc2e67e37512977a71858eda3ba/pydicom/filewriter.py#L1049): [E303](https://duckduckgo.com/?q=pep8%20E303) too many blank lines (2)







Patch:
diff --git a/doc/old/image_data_compression.rst b/doc/old/image_data_compression.rst
index bdfc787f1..c54633731 100644
--- a/doc/old/image_data_compression.rst
+++ b/doc/old/image_data_compression.rst
@@ -23,6 +23,8 @@ The requirements for compressed *Pixel Data* in the DICOM Standard are:
   of encoded data is too large for the basic offset table then the use of
   the :func:`extended offset table <pydicom.encaps.encapsulate_extended>` is
   recommended.
+* A dataset with encapsulated pixel data must use explicit VR little endian
+  encoding
 
 See the :dcm:`relevant sections of the DICOM Standard<part05/sect_8.2.html>`
 for more information.
@@ -49,6 +51,12 @@ for more information.
 
     # Basic encapsulation
     ds.PixelData = encapsulate(frames)
+
+    # Set the dataset encoding
+    ds.is_little_endian = True
+    ds.is_implicit_VR = False
+
+    # Save!
     ds.save_as("CT_small_compressed_basic.dcm")
 
     # Extended encapsulation
@@ -111,6 +119,7 @@ A specific encoding plugin can be used by passing the plugin name via the
 
 .. code-block:: python
 
+    # Will set `ds.is_little_endian` and `ds.is_implicit_VR` automatically
     ds.compress(RLELossless, encoding_plugin='pylibjpeg')
     ds.save_as("CT_small_rle.dcm")
 
diff --git a/doc/release_notes/v2.3.0.rst b/doc/release_notes/v2.3.0.rst
index f9189afef..7f0939097 100644
--- a/doc/release_notes/v2.3.0.rst
+++ b/doc/release_notes/v2.3.0.rst
@@ -26,3 +26,5 @@ Fixes
 
 * Fixed odd-length **OB** values not being padded during write (:issue:`1511`)
 * Fixed Hologic private dictionary entry (0019xx43)
+* Fixed :meth:`~pydicom.dataset.Dataset.compress` not setting the correct
+  encoding for the rest of the dataset (:issue:`1565`)
diff --git a/pydicom/dataset.py b/pydicom/dataset.py
index ffd8197d9..73fe20ce9 100644
--- a/pydicom/dataset.py
+++ b/pydicom/dataset.py
@@ -1712,8 +1712,13 @@ class Dataset:
         else:
             self.PixelData = encapsulate(encoded)
 
+        # PS3.5 Annex A.4 - encapsulated pixel data uses undefined length
         self['PixelData'].is_undefined_length = True
 
+        # PS3.5 Annex A.4 - encapsulated datasets use explicit VR little endian
+        self.is_implicit_VR = False
+        self.is_little_endian = True
+
         # Set the correct *Transfer Syntax UID*
         if not hasattr(self, 'file_meta'):
             self.file_meta = FileMetaDataset()
diff --git a/pydicom/filewriter.py b/pydicom/filewriter.py
index 06f0d54a6..e69aafbd8 100644
--- a/pydicom/filewriter.py
+++ b/pydicom/filewriter.py
@@ -1024,33 +1024,41 @@ def dcmwrite(
         ``save_as()`` wraps ``dcmwrite()``.
     """
     tsyntax: Optional[UID]
+    try:
+        tsyntax = dataset.file_meta.TransferSyntaxUID
+    except AttributeError:
+        tsyntax = None
 
-    # Ensure is_little_endian and is_implicit_VR are set
-    if None in (dataset.is_little_endian, dataset.is_implicit_VR):
-        has_tsyntax = False
-        try:
-            tsyntax = dataset.file_meta.TransferSyntaxUID
-            if not tsyntax.is_private:
-                dataset.is_little_endian = tsyntax.is_little_endian
-                dataset.is_implicit_VR = tsyntax.is_implicit_VR
-                has_tsyntax = True
-        except AttributeError:
-            pass
+    cls_name = dataset.__class__.__name__
+    encoding = (dataset.is_implicit_VR, dataset.is_little_endian)
 
-        if not has_tsyntax:
-            name = dataset.__class__.__name__
+    # Ensure is_little_endian and is_implicit_VR are set
+    if None in encoding:
+        if tsyntax is None:
             raise AttributeError(
-                f"'{name}.is_little_endian' and '{name}.is_implicit_VR' must "
-                f"be set appropriately before saving"
+                f"'{cls_name}.is_little_endian' and "
+                f"'{cls_name}.is_implicit_VR' must be set appropriately "
+                "before saving"
             )
 
-    # Try and ensure that `is_undefined_length` is set correctly
-    try:
-        tsyntax = dataset.file_meta.TransferSyntaxUID
         if not tsyntax.is_private:
+            dataset.is_little_endian = tsyntax.is_little_endian
+            dataset.is_implicit_VR = tsyntax.is_implicit_VR
+
+    if tsyntax and not tsyntax.is_private:
+        # PS3.5 Annex A.4 - the length of encapsulated pixel data is undefined
+        #   and native pixel data uses actual length
+        if "PixelData" in dataset:
             dataset['PixelData'].is_undefined_length = tsyntax.is_compressed
-    except (AttributeError, KeyError):
-        pass
+
+        # PS3.5 Annex A.4 - encapsulated datasets use Explicit VR Little
+        if tsyntax.is_compressed and encoding != (False, True):
+            warnings.warn(
+                "All encapsulated (compressed) transfer syntaxes must use "
+                "explicit VR little endian encoding for the dataset. Set "
+                f"'{cls_name}.is_little_endian = True' and '{cls_name}."
+                "is_implicit_VR = False' before saving"
+            )
 
     # Check that dataset's group 0x0002 elements are only present in the
     #   `dataset.file_meta` Dataset - user may have added them to the wrong


Test patch:
diff --git a/pydicom/tests/test_encoders.py b/pydicom/tests/test_encoders.py
index 98017088c..471d67059 100644
--- a/pydicom/tests/test_encoders.py
+++ b/pydicom/tests/test_encoders.py
@@ -1023,6 +1023,8 @@ class TestDatasetCompress:
         assert len(ds.PixelData) == 21370
         assert 'PlanarConfiguration' not in ds
         assert ds['PixelData'].is_undefined_length
+        assert not ds.is_implicit_VR
+        assert ds.is_little_endian
 
     @pytest.mark.skipif(not HAVE_NP, reason="Numpy is unavailable")
     def test_compress_arr(self):
@@ -1030,11 +1032,16 @@ class TestDatasetCompress:
         ds = get_testdata_file("CT_small.dcm", read=True)
         assert hasattr(ds, 'file_meta')
         arr = ds.pixel_array
+        ds.is_implicit_VR = True
+        assert ds.is_little_endian
         del ds.PixelData
         del ds.file_meta
+
         ds.compress(RLELossless, arr, encoding_plugin='pydicom')
         assert ds.file_meta.TransferSyntaxUID == RLELossless
         assert len(ds.PixelData) == 21370
+        assert not ds.is_implicit_VR
+        assert ds.is_little_endian
 
     @pytest.mark.skipif(HAVE_NP, reason="Numpy is available")
     def test_encoder_unavailable(self, monkeypatch):
diff --git a/pydicom/tests/test_filewriter.py b/pydicom/tests/test_filewriter.py
index 4e251b4c9..29dded13d 100644
--- a/pydicom/tests/test_filewriter.py
+++ b/pydicom/tests/test_filewriter.py
@@ -25,12 +25,16 @@ from pydicom.filereader import dcmread, read_dataset
 from pydicom.filewriter import (
     write_data_element, write_dataset, correct_ambiguous_vr,
     write_file_meta_info, correct_ambiguous_vr_element, write_numbers,
-    write_PN, _format_DT, write_text, write_OWvalue, writers
+    write_PN, _format_DT, write_text, write_OWvalue, writers, dcmwrite
 )
 from pydicom.multival import MultiValue
 from pydicom.sequence import Sequence
-from pydicom.uid import (ImplicitVRLittleEndian, ExplicitVRBigEndian,
-                         PYDICOM_IMPLEMENTATION_UID)
+from pydicom.uid import (
+    ImplicitVRLittleEndian,
+    ExplicitVRBigEndian,
+    RLELossless,
+    PYDICOM_IMPLEMENTATION_UID,
+)
 from pydicom.util.hexutil import hex2bytes
 from pydicom.valuerep import DA, DT, TM, VR
 from pydicom.values import convert_text
@@ -280,8 +284,25 @@ class TestWriteFile:
             r"be set appropriately before saving"
         )
         with pytest.raises(AttributeError, match=msg):
-            write_dataset(bs, ds)
+            dcmwrite(bs, ds)
+
+    def test_write_encapsulated_mismatch_encoding(self):
+        """Test writing encapsulated dataset warns if mismatched encoding"""
+        ds = Dataset()
+        ds.file_meta = FileMetaDataset()
+        ds.file_meta.TransferSyntaxUID = RLELossless
+        ds.is_implicit_VR = True
+        ds.is_little_endian = True
+        bs = BytesIO()
 
+        msg = (
+            r"All encapsulated \(compressed\) transfer syntaxes must use "
+            r"explicit VR little endian encoding for the dataset. Set "
+            r"'Dataset.is_little_endian = True' and 'Dataset."
+            r"is_implicit_VR = False' before saving"
+        )
+        with pytest.warns(UserWarning, match=msg):
+            dcmwrite(bs, ds)
 
 class TestScratchWriteDateTime(TestWriteFile):
     """Write and reread simple or multi-value DA/DT/TM data elements"""