Repository: pydicom/pydicom

Issue:
Intermittent test failures
**tests/test_fileset.py::TestFileSet::test_add_dataset**

https://github.com/pydicom/pydicom/actions/runs/7323474392/job/19946089588

```
assert 398 == (ds.OffsetOfTheFirstDirectoryRecordOfTheRootDirectoryEntity)
E       AssertionError: assert 398 == 396
```
???

This is basically the length of the preceeded encoding dataset (128 + 4, file meta, + 3 elements), which shouldn't be intermittently changing...

Hmm, ah! The UID lengths are probably not necessarily always 64 chars now? Annoying... 

**tests/test_encoders.py::TestDatasetCompress::test_round_trip**

Probably a memory re-use issue

Discussion:
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/pydicom/pydicom/pull/1988?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom) Report
Attention: `4 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`76c565d`)](https://app.codecov.io/gh/pydicom/pydicom/commit/76c565d5582cb44eb975e020f25b9f306b4177f6?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom) 97.93% compared to head [(`d95840f`)](https://app.codecov.io/gh/pydicom/pydicom/pull/1988?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom) 97.90%.

| [Files](https://app.codecov.io/gh/pydicom/pydicom/pull/1988?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom) | Patch % | Lines |
|---|---|---|
| [src/pydicom/dataset.py](https://app.codecov.io/gh/pydicom/pydicom/pull/1988?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom#diff-c3JjL3B5ZGljb20vZGF0YXNldC5weQ==) | 77.77% | [4 Missing :warning: ](https://app.codecov.io/gh/pydicom/pydicom/pull/1988?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main    #1988      +/-   ##
==========================================
- Coverage   97.93%   97.90%   -0.04%     
==========================================
  Files          63       63              
  Lines       10816    10833      +17     
==========================================
+ Hits        10593    10606      +13     
- Misses        223      227       +4     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/pydicom/pydicom/pull/1988?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom).


Patch:
diff --git a/doc/release_notes/v3.0.0.rst b/doc/release_notes/v3.0.0.rst
index 77f631453..799a9ac22 100644
--- a/doc/release_notes/v3.0.0.rst
+++ b/doc/release_notes/v3.0.0.rst
@@ -178,6 +178,9 @@ Fixes
   explicit and implicit VR when only the *Transfer Syntax UID* was changed (:issue:`1943`)
 * Fixed the ``jpeg_ls``, ``pillow`` and ``rle`` pixel data handlers not working
   correctly when a frame is spread across multiple fragments (:issue:`1774`)
+* Added mitigation for a rare case where clearing the pixel data value prior
+  to updating it may sometimes result in :attr:`~pydicom.dataset.Dataset.pixel_array`
+  returning the previous array instead of creating a new one (:issue:`1983`)
 * Fixed a ``KeyError`` when comparing codes with one of the codes having
   ``scheme_designator`` set to ``SRT`` but not being included in the ``SRT``
   to ``SCT`` code mapping (:issue:`1994`)
diff --git a/src/pydicom/dataset.py b/src/pydicom/dataset.py
index d99f0694f..ab8566c21 100644
--- a/src/pydicom/dataset.py
+++ b/src/pydicom/dataset.py
@@ -84,6 +84,10 @@ from pydicom.valuerep import VR as VR_, AMBIGUOUS_VR
 from pydicom.waveforms import numpy_handler as wave_handler
 
 
+# FloatPixelData, DoubleFloatPixelData, PixelData
+PIXEL_KEYWORDS = {0x7FE00008, 0x7FE00009, 0x7FE00010}
+
+
 class PrivateBlock:
     """Helper class for a private block in the :class:`Dataset`.
 
@@ -647,6 +651,11 @@ class Dataset:
         tag = cast(BaseTag, tag_for_keyword(name))
         if tag is not None and tag in self._dict:
             del self._dict[tag]
+
+            # Deleting pixel data resets the stored array
+            if tag in PIXEL_KEYWORDS:
+                self._pixel_array = None
+                self._pixel_id = {}
         # If not a DICOM name in this dataset, check for regular instance name
         #   can't do delete directly, that will call __delattr__ again
         elif name in self.__dict__:
@@ -693,10 +702,19 @@ class Dataset:
                 # deleted - will be re-created on next access
                 if self._private_blocks and BaseTag(tag).is_private_creator:
                     self._private_blocks = {}
+
+                if tag in PIXEL_KEYWORDS:
+                    self._pixel_array = None
+                    self._pixel_id = {}
         elif isinstance(key, BaseTag):
             del self._dict[key]
             if self._private_blocks and key.is_private_creator:
                 self._private_blocks = {}
+
+            # Deleting pixel data resets the stored array
+            if key in PIXEL_KEYWORDS:
+                self._pixel_array = None
+                self._pixel_id = {}
         else:
             # If not a standard tag, than convert to Tag and try again
             tag = Tag(key)
@@ -704,6 +722,11 @@ class Dataset:
             if self._private_blocks and tag.is_private_creator:
                 self._private_blocks = {}
 
+            # Deleting pixel data resets the stored array
+            if tag in PIXEL_KEYWORDS:
+                self._pixel_array = None
+                self._pixel_id = {}
+
     def __dir__(self) -> list[str]:
         """Return a list of methods, properties, attributes and element
         keywords available in the :class:`Dataset`.
@@ -1650,7 +1673,14 @@ class Dataset:
         already_have = True
         if not hasattr(self, "_pixel_array"):
             already_have = False
-        elif self._pixel_id != get_image_pixel_ids(self):
+        elif self._pixel_array is None:
+            already_have = False
+
+        # Chcking `_pixel_id` may sometimes give a false result if the pixel
+        #   data memory has been freed (such as with ds.PixelData = None)
+        #   prior to setting a new value; Python may reuse that freed memory
+        #   for the new value and therefore give the same `id()` value
+        if self._pixel_id != get_image_pixel_ids(self):
             already_have = False
 
         if already_have:
@@ -1925,6 +1955,8 @@ class Dataset:
 
         # PS3.5 Annex A.4 - encapsulated pixel data uses undefined length
         self["PixelData"].is_undefined_length = True
+        self._pixel_array = None
+        self._pixel_id = {}
 
         # Set the correct *Transfer Syntax UID*
         if not hasattr(self, "file_meta"):
@@ -2519,6 +2551,11 @@ class Dataset:
                     elem = DataElement_from_raw(elem, self._character_set, self)
                 elem.private_creator = self[private_creator_tag].value
 
+        # Changing pixel data resets the stored array
+        if elem_tag in PIXEL_KEYWORDS:
+            self._pixel_array = None
+            self._pixel_id = {}
+
         self._dict[elem_tag] = elem
 
         if elem.VR == VR_.SQ and isinstance(elem, DataElement):


Test patch:
diff --git a/tests/test_dataset.py b/tests/test_dataset.py
index 4f91285a2..13210f660 100644
--- a/tests/test_dataset.py
+++ b/tests/test_dataset.py
@@ -1415,6 +1415,51 @@ class TestDataset:
 
         pydicom.config.pixel_data_handlers = orig_handlers
 
+    def test_pixel_array_id_reset_on_delete(self):
+        """Test _pixel_array and _pixel_id reset when deleting pixel data"""
+        fpath = get_testdata_file("CT_small.dcm")
+        ds = dcmread(fpath)
+        assert ds._pixel_id == {}
+        assert ds._pixel_array is None
+        ds._pixel_id = {"SamplesPerPixel": 3}
+        ds._pixel_array = True
+
+        del ds.PixelData
+        assert ds._pixel_id == {}
+        assert ds._pixel_array is None
+
+        ds.PixelData = b"\x00\x01"
+        ds._pixel_id = {"SamplesPerPixel": 3}
+        ds._pixel_array = True
+
+        del ds["PixelData"]
+        assert ds._pixel_id == {}
+        assert ds._pixel_array is None
+
+        ds.PixelData = b"\x00\x01"
+        ds._pixel_id = {"SamplesPerPixel": 3}
+        ds._pixel_array = True
+
+        del ds[Tag("PixelData")]
+        assert ds._pixel_id == {}
+        assert ds._pixel_array is None
+
+        ds.PixelData = b"\x00\x01"
+        ds._pixel_id = {"SamplesPerPixel": 3}
+        ds._pixel_array = True
+
+        del ds[0x7FE00010]
+        assert ds._pixel_id == {}
+        assert ds._pixel_array is None
+
+        ds.PixelData = b"\x00\x01"
+        ds._pixel_id = {"SamplesPerPixel": 3}
+        ds._pixel_array = True
+
+        del ds[0x7FE00000:0x7FE00012]
+        assert ds._pixel_id == {}
+        assert ds._pixel_array is None
+
     def test_pixel_array_unknown_syntax(self):
         """Test that pixel_array for an unknown syntax raises exception."""
         ds = dcmread(get_testdata_file("CT_small.dcm"))
diff --git a/tests/test_encoders.py b/tests/test_encoders.py
index 1aaa5cb6c..d3e3f5a22 100644
--- a/tests/test_encoders.py
+++ b/tests/test_encoders.py
@@ -1055,11 +1055,11 @@ class TestDatasetCompress:
         """Test an encoding round-trip"""
         ds = get_testdata_file("MR_small_RLE.dcm", read=True)
         arr = ds.pixel_array
+        # Setting PixelData to None frees the memory which may
+        #   sometimes be reused, causes the _pixel_id check to fail
         ds.PixelData = None
         ds._pixel_array = None
         ds.compress(RLELossless, arr, encoding_plugin="pydicom")
-        # `pixel_array` will be regenerated
-        assert ds._pixel_array is None
         assert ds.PixelData is not None
         assert np.array_equal(arr, ds.pixel_array)