Repository: pydicom/pydicom

Issue:
Unexpected behavior on re-adding a private tag to a dataset after remove_private_tags
I've encountered this issue while trying to write a solution to #1087.
I'm trying to add a past private tag to a dataset that had all his private tags removed with remove_private_tags. It leads to a single element update - just the private tag data. No private creator element is added. Also, the private tag is updated on the second free block rather than the first - that's supposed to be free after remove_private_tags.
It seems the dataset preserves some information on past private creators blocks even after remove_private_tags. 

I expect that a private creator element would be (re-)created if I'm trying to add a private tag, and that it would created on the first available place in the private group.

Reproducing code:
```
from pydicom import Dataset

ds = Dataset()
block = ds.private_block(0x000b, 'dog^1', create=True)
block.add_new(0x01, "SH", "Border Collie")
block = ds.private_block(0x000b, 'dog^2', create=True)
block.add_new(0x01, "SH", "Poodle")

ds.remove_private_tags()
block = ds.private_block(0x000b, 'dog^2', create=True)
block.add_new(0x01, "SH", "Poodle")
print(ds)

```
Output:
```
(000b, 1101) Private tag data                    SH: 'Poodle'`
```

Aa far as I understand, output should be:
```
(000b, 0010) Private Creator                     LO: 'dog^2'
(000b, 1001) Private tag data                    SH: 'Poodle'
```

My environment (Running with docker from PyCharm):
Linux-4.15.0-74-generic-x86_64-with-debian-buster-sid
Python 3.6.9 |Anaconda, Inc.| (default, Jul 30 2019, 19:07:31)
[GCC 7.3.0]
pydicom 2.0.0.dev
```

Discussion:

Patch:
diff --git a/doc/release_notes/v2.0.0.rst b/doc/release_notes/v2.0.0.rst
index ae211abd9..20c2325a5 100644
--- a/doc/release_notes/v2.0.0.rst
+++ b/doc/release_notes/v2.0.0.rst
@@ -36,5 +36,6 @@ Fixes
   (0028,3006) *LUT Data* with a VR of **OW** (:issue:`1073`)
 * Fixed access to private creator tag in raw datasets (:issue:`1078`)
 * Fixed description of newly added known private tag (:issue:`1082`)
+* Fixed update of private blocks after deleting private creator (:issue:`1097`)
 * Fixed bug in updating `pydicom.config.use_DS_Decimal` flag
   in :func:`~pydicom.config.DS_decimal`
diff --git a/pydicom/dataset.py b/pydicom/dataset.py
index ec8f68196..da486df83 100644
--- a/pydicom/dataset.py
+++ b/pydicom/dataset.py
@@ -581,14 +581,22 @@ class Dataset(dict):
         if isinstance(key, slice):
             for tag in self._slice_dataset(key.start, key.stop, key.step):
                 del self._dict[tag]
+                # invalidate private blocks in case a private creator is
+                # deleted - will be re-created on next access
+                if self._private_blocks and BaseTag(tag).is_private_creator:
+                    self._private_blocks = {}
         else:
             # Assume is a standard tag (for speed in common case)
             try:
                 del self._dict[key]
+                if self._private_blocks and BaseTag(key).is_private_creator:
+                    self._private_blocks = {}
             # If not a standard tag, than convert to Tag and try again
             except KeyError:
                 tag = Tag(key)
                 del self._dict[tag]
+                if self._private_blocks and tag.is_private_creator:
+                    self._private_blocks = {}
 
     def __dir__(self):
         """Give a list of attributes available in the :class:`Dataset`.
@@ -905,7 +913,7 @@ class Dataset(dict):
             If the private creator tag is not found in the given group and
             the `create` parameter is ``False``.
         """
-        def new_block():
+        def new_block(element):
             block = PrivateBlock(key, self, element)
             self._private_blocks[key] = block
             return block
@@ -921,19 +929,23 @@ class Dataset(dict):
             raise ValueError(
                 'Tag must be private if private creator is given')
 
-        for element in range(0x10, 0x100):
-            private_creator_tag = Tag(group, element)
-            if private_creator_tag not in self._dict:
-                if create:
-                    self.add_new(private_creator_tag, 'LO', private_creator)
-                    return new_block()
-                else:
-                    break
-            if self[private_creator_tag].value == private_creator:
-                return new_block()
+        # find block with matching private creator
+        data_el = next((el for el in self[(group, 0x10):(group, 0x100)]
+                        if el.value == private_creator), None)
+        if data_el is not None:
+            return new_block(data_el.tag.element)
+
+        if not create:
+            # not found and shall not be created - raise
+            raise KeyError(
+                "Private creator '{}' not found".format(private_creator))
 
-        raise KeyError(
-            "Private creator '{}' not found".format(private_creator))
+        # private creator not existing - find first unused private block
+        # and add the private creator
+        first_free_el = next(el for el in range(0x10, 0x100)
+                             if Tag(group, el) not in self._dict)
+        self.add_new(Tag(group, first_free_el), 'LO', private_creator)
+        return new_block(first_free_el)
 
     def private_creators(self, group):
         """Return a list of private creator names in the given group.
@@ -967,13 +979,7 @@ class Dataset(dict):
         if group % 2 == 0:
             raise ValueError('Group must be an odd number')
 
-        private_creators = []
-        for element in range(0x10, 0x100):
-            private_creator_tag = Tag(group, element)
-            if private_creator_tag not in self._dict:
-                break
-            private_creators.append(self[private_creator_tag].value)
-        return private_creators
+        return [x.value for x in self[(group, 0x10):(group, 0x100)]]
 
     def get_private_item(self, group, element_offset, private_creator):
         """Return the data element for the given private tag `group`.


Test patch:
diff --git a/pydicom/tests/test_dataset.py b/pydicom/tests/test_dataset.py
index 78e2083d1..d3efc2079 100644
--- a/pydicom/tests/test_dataset.py
+++ b/pydicom/tests/test_dataset.py
@@ -917,9 +917,10 @@ class TestDataset(object):
         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(0x00090011, 'LO', 'Creator 2.0')
-        ds.add_new(0x00091101, 'SH', 'Version2')
-        ds.add_new(0x00091102, 'US', 2)
+        # make sure it works with non-contiguous blocks
+        ds.add_new(0x00090020, 'LO', 'Creator 2.0')
+        ds.add_new(0x00092001, 'SH', 'Version2')
+        ds.add_new(0x00092002, 'US', 2)
 
         # Dataset.private_block
         with pytest.raises(ValueError, match='Tag must be private'):
@@ -1019,6 +1020,68 @@ class TestDataset(object):
         assert ['Creator 1.0', 'Creator 2.0'] == ds.private_creators(0x0009)
         assert not ds.private_creators(0x0011)
 
+    def test_non_contiguous_private_creators(self):
+        ds = Dataset()
+        ds.add_new(0x00080005, 'CS', 'ISO_IR 100')
+        ds.add_new(0x00090010, 'LO', 'Creator 1.0')
+        ds.add_new(0x00090020, 'LO', 'Creator 2.0')
+        ds.add_new(0x000900ff, 'LO', 'Creator 3.0')
+
+        assert (['Creator 1.0', 'Creator 2.0', 'Creator 3.0'] ==
+                ds.private_creators(0x0009))
+
+    def test_create_private_tag_after_removing_all(self):
+        # regression test for #1097 - make sure private blocks are updated
+        # after removing all private tags
+        ds = Dataset()
+        block = ds.private_block(0x000b, 'dog^1', create=True)
+        block.add_new(0x01, "SH", "Border Collie")
+        block = ds.private_block(0x000b, 'dog^2', create=True)
+        block.add_new(0x01, "SH", "Poodle")
+
+        ds.remove_private_tags()
+        block = ds.private_block(0x000b, 'dog^2', create=True)
+        block.add_new(0x01, "SH", "Poodle")
+        assert len(ds) == 2
+        assert (0x000b0010) in ds
+        assert ds[0x000b0010].value == 'dog^2'
+
+    def test_create_private_tag_after_removing_private_creator(self):
+        ds = Dataset()
+        block = ds.private_block(0x000b, 'dog^1', create=True)
+        block.add_new(0x01, "SH", "Border Collie")
+
+        del ds[0x000b0010]
+        block = ds.private_block(0x000b, 'dog^1', create=True)
+        block.add_new(0x02, "SH", "Poodle")
+        assert len(ds) == 3
+        assert ds[0x000b0010].value == 'dog^1'
+
+        del ds[Tag(0x000b, 0x0010)]
+        block = ds.private_block(0x000b, 'dog^1', create=True)
+        block.add_new(0x01, "SH", "Pug")
+        assert len(ds) == 3
+        assert ds[0x000b0010].value == 'dog^1'
+
+        del ds['0x000b0010']
+        block = ds.private_block(0x000b, 'dog^1', create=True)
+        block.add_new(0x01, "SH", "Pug")
+        assert len(ds) == 3
+        assert ds[0x000b0010].value == 'dog^1'
+
+    def test_create_private_tag_after_removing_slice(self):
+        ds = Dataset()
+        block = ds.private_block(0x000b, 'dog^1', create=True)
+        block.add_new(0x01, "SH", "Border Collie")
+        block = ds.private_block(0x000b, 'dog^2', create=True)
+        block.add_new(0x01, "SH", "Poodle")
+
+        del ds[0x000b0010:0x000b1110]
+        block = ds.private_block(0x000b, 'dog^2', create=True)
+        block.add_new(0x01, "SH", "Poodle")
+        assert len(ds) == 2
+        assert ds[0x000b0010].value == 'dog^2'
+
     def test_is_original_encoding(self):
         """Test Dataset.write_like_original"""
         ds = Dataset()