Repository: pydicom/pydicom

Issue:
Dataset.copy returns an empty dict
**Describe the bug**
Call to Dataset.copy on an instance of a Dataset object returns an empty python dict.

**Expected behavior**
A shallow copy of the callee

**Steps To Reproduce**
```python
>>> from pydicom import Dataset
>>> ds = Dataset()
>>> ds.PatientName = "Jhon^Doe"
>>> ds
(0010, 0010) Patient's Name                      PN: 'Jhon^Doe'
>>> ds_copy = ds.copy()
>>> ds_copy
{}
>>> 
```


**Your environment**
```python
>>> import platform, sys, pydicom
>>> print(platform.platform(),
...       "\nPython", sys.version,
...       "\npydicom", pydicom.__version__)
Linux-5.4.0-40-generic-x86_64-with-debian-bullseye-sid 
Python 3.7.6 (default, May 20 2020, 16:52:50) 
[GCC 9.3.0] 
pydicom 1.4.2
```

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/pydicom/pydicom/pull/1151?src=pr&el=h1) Report
> Merging [#1151](https://codecov.io/gh/pydicom/pydicom/pull/1151?src=pr&el=desc) into [master](https://codecov.io/gh/pydicom/pydicom/commit/5c4e74e77f0a384c3dedb1678fbe02c60575d70e&el=desc) will **decrease** coverage by `0.10%`.
> The diff coverage is `87.65%`.

[![Impacted file tree graph](https://codecov.io/gh/pydicom/pydicom/pull/1151/graphs/tree.svg?width=650&height=150&src=pr&token=YciLV15Lwr)](https://codecov.io/gh/pydicom/pydicom/pull/1151?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1151      +/-   ##
==========================================
- Coverage   94.95%   94.85%   -0.11%     
==========================================
  Files          91       93       +2     
  Lines       18924    19094     +170     
==========================================
+ Hits        17970    18111     +141     
- Misses        954      983      +29     
```


| [Impacted Files](https://codecov.io/gh/pydicom/pydicom/pull/1151?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pydicom/sr/codedict.py](https://codecov.io/gh/pydicom/pydicom/pull/1151/diff?src=pr&el=tree#diff-cHlkaWNvbS9zci9jb2RlZGljdC5weQ==) | `63.10% <50.00%> (+1.28%)` | :arrow_up: |
| [pydicom/data/retry.py](https://codecov.io/gh/pydicom/pydicom/pull/1151/diff?src=pr&el=tree#diff-cHlkaWNvbS9kYXRhL3JldHJ5LnB5) | `57.14% <57.14%> (ø)` | |
| [pydicom/filereader.py](https://codecov.io/gh/pydicom/pydicom/pull/1151/diff?src=pr&el=tree#diff-cHlkaWNvbS9maWxlcmVhZGVyLnB5) | `92.95% <66.66%> (ø)` | |
| [pydicom/data/download.py](https://codecov.io/gh/pydicom/pydicom/pull/1151/diff?src=pr&el=tree#diff-cHlkaWNvbS9kYXRhL2Rvd25sb2FkLnB5) | `76.63% <76.63%> (ø)` | |
| [pydicom/charset.py](https://codecov.io/gh/pydicom/pydicom/pull/1151/diff?src=pr&el=tree#diff-cHlkaWNvbS9jaGFyc2V0LnB5) | `94.07% <83.33%> (ø)` | |
| [pydicom/filewriter.py](https://codecov.io/gh/pydicom/pydicom/pull/1151/diff?src=pr&el=tree#diff-cHlkaWNvbS9maWxld3JpdGVyLnB5) | `97.08% <91.30%> (+0.28%)` | :arrow_up: |
| [pydicom/data/data\_manager.py](https://codecov.io/gh/pydicom/pydicom/pull/1151/diff?src=pr&el=tree#diff-cHlkaWNvbS9kYXRhL2RhdGFfbWFuYWdlci5weQ==) | `100.00% <100.00%> (ø)` | |
| [pydicom/dataset.py](https://codecov.io/gh/pydicom/pydicom/pull/1151/diff?src=pr&el=tree#diff-cHlkaWNvbS9kYXRhc2V0LnB5) | `98.73% <100.00%> (+<0.01%)` | :arrow_up: |
| [pydicom/tests/test\_charset.py](https://codecov.io/gh/pydicom/pydicom/pull/1151/diff?src=pr&el=tree#diff-cHlkaWNvbS90ZXN0cy90ZXN0X2NoYXJzZXQucHk=) | `100.00% <100.00%> (ø)` | |
| [pydicom/tests/test\_data\_manager.py](https://codecov.io/gh/pydicom/pydicom/pull/1151/diff?src=pr&el=tree#diff-cHlkaWNvbS90ZXN0cy90ZXN0X2RhdGFfbWFuYWdlci5weQ==) | `100.00% <100.00%> (ø)` | |
| ... and [13 more](https://codecov.io/gh/pydicom/pydicom/pull/1151/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pydicom/pydicom/pull/1151?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pydicom/pydicom/pull/1151?src=pr&el=footer). Last update [b9fb05c...48f46b6](https://codecov.io/gh/pydicom/pydicom/pull/1151?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

mrbean-bremen: I have been thinking about this, and my gut feeling is to leave it as it is, e.g. `Dataset.copy` just doing a shallow copy as in `dict`. I don't think we shall change the functionality of `copy.copy` and `copy.deepcopy`. For `Dataset.copy` one could argue, that the expected behavior is a deep copy (I remember a related discussion), or we could add an additional `Dataset.deepcopy`.

What do you think? 
darcymason: > What do you think?

Oops - typed my answer in the review conversation.
darcymason: > and my gut feeling is to leave it as it is, e.g. `Dataset.copy` just doing a shallow copy as in `dict`. I don't think we shall change the functionality of `copy.copy` and `copy.deepcopy`. For `Dataset.copy` one could argue though, that the expected behavior is a deep copy (I remember a related discussion), or we could add an additional `Dataset.deepcopy`

Sorry - I'm catching up after a complete hardware failure on my laptop - should have consolidated all my answers. ...  Just to address this part a little more specifically - I agree to leave ds.copy() as shallow as in `dict`.  I don't remember the details of the related conversation but I think adherence to Python standard way should be a top principle (as per iter discussion, it always causes confusion when we deviate).  I think `copy` (shallow) in Python is a 'gotcha' which causes unexpected behavior for many people (change some attribute and it changes in the copy too), but it is what it is and I think it is defensible to 'copy' that behavior in pydicom.
mrbean-bremen: > Sorry - I'm catching up after a complete hardware failure on my laptop

Argh - yes, I had that last year... had to install a new SSD. And of course didn't backup all data before...
darcymason: > > Sorry - I'm catching up after a complete hardware failure on my laptop
> 
> Argh - yes, I had that last year... had to install a new SSD. And of course didn't backup all data before...

Mine was a complete lack of anything happening on pushing the power button.  Eventually came back after some magic combination of resetting internal battery, then powering on while plugged in without the main battery, etc.... not sure if what I did actually solved it or whether it will randomly reappear.


Patch:
diff --git a/doc/release_notes/v2.1.0.rst b/doc/release_notes/v2.1.0.rst
index f8d89bda0..b7cfca5bd 100644
--- a/doc/release_notes/v2.1.0.rst
+++ b/doc/release_notes/v2.1.0.rst
@@ -21,3 +21,4 @@ Enhancements
 
 Fixes
 .....
+* ``Dataset.copy`` now works as expected (see :issue:`1146`)
\ No newline at end of file
diff --git a/pydicom/dataset.py b/pydicom/dataset.py
index 8274c97cb..abe3de5a4 100644
--- a/pydicom/dataset.py
+++ b/pydicom/dataset.py
@@ -14,7 +14,7 @@ Dataset (dict subclass)
         * A Sequence (list subclass), where each item is a Dataset which
             contains its own DataElements, and so on in a recursive manner.
 """
-
+import copy
 from bisect import bisect_left
 import io
 import inspect  # for __dir__
@@ -502,6 +502,10 @@ class Dataset(dict):
 
         self.walk(decode_callback, recursive=False)
 
+    def copy(self):
+        """Return a shallow copy of the dataset."""
+        return copy.copy(self)
+
     def __delattr__(self, name):
         """Intercept requests to delete an attribute by `name`.
 


Test patch:
diff --git a/pydicom/tests/test_dataset.py b/pydicom/tests/test_dataset.py
index fc186273e..38cffa086 100644
--- a/pydicom/tests/test_dataset.py
+++ b/pydicom/tests/test_dataset.py
@@ -1,16 +1,17 @@
 # Copyright 2008-2018 pydicom authors. See LICENSE file for details.
 """Unit tests for the pydicom.dataset module."""
+import copy
 
 import pytest
 
 import pydicom
+from pydicom import dcmread
 from pydicom.data import get_testdata_file
 from pydicom.dataelem import DataElement, RawDataElement
 from pydicom.dataset import (
     Dataset, FileDataset, validate_file_meta, FileMetaDataset
 )
 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
 from pydicom.pixel_data_handlers.util import get_image_pixel_ids
@@ -1834,3 +1835,29 @@ class TestFileMeta:
         assert shown.startswith("(0010, 0010) Patient's Name")
 
         pydicom.config.show_file_meta = orig_show
+
+    @pytest.mark.parametrize('copy_method',
+                             [Dataset.copy, copy.copy, copy.deepcopy])
+    def test_copy(self, copy_method):
+        ds = Dataset()
+        ds.PatientName = "John^Doe"
+        ds.BeamSequence = [Dataset(), Dataset(), Dataset()]
+        ds.BeamSequence[0].Manufacturer = "Linac, co."
+        ds.BeamSequence[1].Manufacturer = "Linac and Sons, co."
+        ds.is_implicit_VR = True
+        ds.is_little_endian = True
+        ds.read_encoding = "utf-8"
+        ds_copy = copy_method(ds)
+        assert isinstance(ds_copy, Dataset)
+        assert len(ds_copy) == 2
+        assert ds_copy.PatientName == "John^Doe"
+        assert ds_copy.BeamSequence[0].Manufacturer == "Linac, co."
+        assert ds_copy.BeamSequence[1].Manufacturer == "Linac and Sons, co."
+        if copy_method == copy.deepcopy:
+            assert id(ds_copy.BeamSequence[0]) != id(ds.BeamSequence[0])
+        else:
+            # shallow copy
+            assert id(ds_copy.BeamSequence[0]) == id(ds.BeamSequence[0])
+        assert ds_copy.is_implicit_VR
+        assert ds_copy.is_little_endian
+        assert ds_copy.read_encoding == "utf-8"