Repository: pydicom/pydicom

Issue:
Option to raise FileExistsError when writing a DICOM file
```
OS version: Windows 11 Pro 23H2 Build 22631.3737
Python version: 3.11.8
Pydicom version: 2.4.4
```

**Is your feature request related to a problem? Please describe.**

When writing a DICOM file, pydicom overwrites any file existing at the same path instead of raising a `FileExistsError`. Not only does this violate Python conventions, there is no way to raise it without using an `if` conditional, which is not useful in all situations, such as mine and is in-fact, not the point of the Forgiveness-Not-Permission paradigm that Python employs.

For example:
```python
from pathlib import Path

import pydicom


dirpath = Path(<some path>)
dicom_file_in_1 = pydicom.read_file(dirpath / "2.6.8.1.0.4.36070.2.1.1.11320856655815744006306524840001.dcm")
dicom_file_in_2 = pydicom.read_file(dirpath /  "2.6.8.1.0.4.36070.2.1.1.11320856655815744006306524840002.dcm")

pydicom.write_file(
    filename=Path.cwd() / "test.dcm",
    dataset=dicom_file_in_1,
    )

pydicom.dcmwrite(
    filename=Path.cwd() / "test.dcm",
    dataset=dicom_file_in_2,
    )

dicom_file_in_1.save_as(filename=Path.cwd() / "test.dcm",)
dicom_file_in_2.save_as(filename=Path.cwd() / "test.dcm",)
```

Now my problem is that I am using multiprocessing to process DICOM files, one process per file. We are using some DICOM header data to name the new file. It is usually unique for DICOM files in the same directory, but in the new datasets we have received, every once in a while it is not.

Since I am using multiprocessing, there is always a possibility of a race condition, where in the time a process checks that a file with same name does not exist, another process has written a different DICOM to the same path, and we lose one of the DICOM files silently.

If there was an option to raise a `FileExistsError`, this would have been trivial to handle, but now we have to add a randomly generated suffix to every processed DICOM file to prevent this, and then run another step after the processing to rename the files appropriately.

**Describe the solution you'd like**
Two ideas off the top of my head, ideally both should be implemented and *backported to older versions*:
1. A parameter `raise_if_exists=False` for all DICOM writing methods and functions.
2. A global option which can be set in `pydicom.config`.

Then if there is an existing file with the same path, it will raise the exception and a suffix added to the name.


Discussion:
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/pydicom/pydicom/pull/2105?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 98.35%. Comparing base [(`9f1e8e1`)](https://app.codecov.io/gh/pydicom/pydicom/commit/9f1e8e132e6b49822f4442b44c00e4415e5c3f39?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom) to head [(`e141850`)](https://app.codecov.io/gh/pydicom/pydicom/commit/e141850f8940fb85eb78447cfe4fe473a232ab9d?dropdown=coverage&el=desc&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    #2105      +/-   ##
==========================================
+ Coverage   98.30%   98.35%   +0.04%     
==========================================
  Files          76       76              
  Lines       13242    13243       +1     
==========================================
+ Hits        13018    13025       +7     
+ Misses        224      218       -6     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/pydicom/pydicom/pull/2105?dropdown=coverage&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).

darcymason: We should discuss the default ... having it now raise will break a _lot_ of old code.  The use case of reading a file, modifying it, and writing it back out is very common, I imagine.   I'd argue that the old behavior should be preserved, and users should take an extra step to avoid overwriting.
mrbean-bremen: > We should discuss the default 

Good point, I hadn't thought about this. I agree that the old behavior should be the default.
scaramallion: >  I'd argue that the old behavior should be preserved

Fair enough!

Patch:
diff --git a/doc/release_notes/v3.0.0.rst b/doc/release_notes/v3.0.0.rst
index 2c315b058..821dab237 100644
--- a/doc/release_notes/v3.0.0.rst
+++ b/doc/release_notes/v3.0.0.rst
@@ -35,6 +35,9 @@ Changes
   method will now raise an exception if the user tries to convert between little
   and big endian datasets. If this is something you need, use
   :func:`~pydicom.filewriter.dcmwrite` instead.
+* Added the `exist_ok` argument to :meth:`Dataset.save_as()<pydicom.dataset.Dataset.save_as>`
+  and :func:`~pydicom.filewriter.dcmwrite` to allow raising a :class:`FileExistsError`
+  if trying to write to a file that already exists (:issue:`2104`).
 * `implicit_vr`, `little_endian` and `force_encoding` optional arguments
   added to  :func:`~pydicom.filewriter.dcmwrite`.
 * The priority used to decide which encoding to use with
@@ -234,6 +237,8 @@ Enhancements
   prior to conversion to a :class:`~pydicom.dataelem.DataElement` (:issue:`1739`).
 * Added support for using :class:`io.BufferedIOBase` subclasses to set the value for
   elements with O* VRs such as **OB** and **OW** (:issue:`1913`).
+* Added elements with **OB**,  **OD**, **OF**, **OL**, **OW**, **OV** VRs to the type
+  validation checking when setting :class:`~pydicom.dataelem.DataElement` values (:issue:`1414`).
 
 
 Fixes
diff --git a/src/pydicom/dataset.py b/src/pydicom/dataset.py
index 661b421fa..98a8e6021 100644
--- a/src/pydicom/dataset.py
+++ b/src/pydicom/dataset.py
@@ -2533,6 +2533,7 @@ class Dataset:
         implicit_vr: bool | None = None,
         little_endian: bool | None = None,
         enforce_file_format: bool = False,
+        exist_ok: bool = True,
         **kwargs: Any,
     ) -> None:
         """Encode the current :class:`Dataset` and write it to `filename`.
@@ -2557,7 +2558,8 @@ class Dataset:
 
         .. versionchanged:: 3.0
 
-            Added `implicit_vr`, `little_endian` and `enforce_file_format`
+            Added `implicit_vr`, `little_endian`, `enforce_file_format` and `exist_ok`
+            keyword arguments
 
         .. deprecated:: 3.0
 
@@ -2599,6 +2601,10 @@ class Dataset:
             - ``Dataset.file_meta``: if the dataset is missing any required
               *File Meta Information Group* elements then they will not be
               added or written
+        exist_ok : bool, optional
+            If ``False`` and `filename` is a :class:`str` or PathLike, then raise a
+            :class:`FileExistsError` if a file already exists with the given filename
+            (default ``True``).
 
         See Also
         --------
@@ -2640,6 +2646,7 @@ class Dataset:
             implicit_vr=implicit_vr,
             little_endian=little_endian,
             enforce_file_format=enforce_file_format,
+            exist_ok=exist_ok,
             **kwargs,
         )
 
diff --git a/src/pydicom/filewriter.py b/src/pydicom/filewriter.py
index efcb740c1..48315b71f 100644
--- a/src/pydicom/filewriter.py
+++ b/src/pydicom/filewriter.py
@@ -1123,6 +1123,7 @@ def dcmwrite(
     little_endian: bool | None = None,
     enforce_file_format: bool = False,
     force_encoding: bool = False,
+    exist_ok: bool = True,
     **kwargs: Any,
 ) -> None:
     """Write `dataset` to `filename`, which can be a path, a file-like or a
@@ -1130,7 +1131,7 @@ def dcmwrite(
 
     .. versionchanged:: 3.0
 
-        Added the `enforce_file_format` keyword argument.
+        Added the `enforce_file_format` and `exist_ok` keyword arguments.
 
     .. deprecated:: 3.0
 
@@ -1258,6 +1259,10 @@ def dcmwrite(
         If ``True`` then force the encoding to follow `implicit_vr` and
         `little_endian`. Cannot be used with `enforce_file_format`. Default
         ``False``.
+    exist_ok : bool, optional
+        If ``False`` and `filename` is a :class:`str` or PathLike, then raise a
+        :class:`FileExistsError` if a file already exists with the given filename
+        (default ``True``).
 
     Raises
     ------
@@ -1402,7 +1407,8 @@ def dcmwrite(
     filename = path_from_pathlike(filename)
     if isinstance(filename, str):
         # A path-like to be written to
-        fp: DicomIO = DicomFile(filename, "wb")
+        file_mode = "xb" if not exist_ok else "wb"
+        fp: DicomIO = DicomFile(filename, file_mode)
         # caller provided a file name; we own the file handle
         caller_owns_file = False
     elif isinstance(filename, DicomIO):
diff --git a/src/pydicom/valuerep.py b/src/pydicom/valuerep.py
index f5b8385c5..e74785e22 100644
--- a/src/pydicom/valuerep.py
+++ b/src/pydicom/valuerep.py
@@ -377,6 +377,12 @@ VALIDATORS = {
     "IS": validate_length_and_type_and_regex,
     "LO": validate_type_and_length,
     "LT": validate_type_and_length,
+    "OB": lambda vr, value: validate_type(vr, value, (bytes, bytearray)),
+    "OD": lambda vr, value: validate_type(vr, value, (bytes, bytearray)),
+    "OF": lambda vr, value: validate_type(vr, value, (bytes, bytearray)),
+    "OL": lambda vr, value: validate_type(vr, value, (bytes, bytearray)),
+    "OW": lambda vr, value: validate_type(vr, value, (bytes, bytearray)),
+    "OV": lambda vr, value: validate_type(vr, value, (bytes, bytearray)),
     "PN": validate_pn,
     "SH": validate_type_and_length,
     "SL": lambda vr, value: validate_number(vr, value, -0x80000000, 0x7FFFFFFF),


Test patch:
diff --git a/tests/test_dataelem.py b/tests/test_dataelem.py
index a4cccd70c..004c0ae35 100644
--- a/tests/test_dataelem.py
+++ b/tests/test_dataelem.py
@@ -1262,6 +1262,25 @@ class TestDataElementValidation:
 
         assert ds.PixelData == b"\x00\x01"
 
+    @pytest.mark.parametrize("value", (None, b"", b"\x00", b"\x00\x01\x02\x03"))
+    def test_valid_o_star_bytes(self, value):
+        for vr in ("OB", "OD", "OF", "OL", "OW", "OV"):
+            DataElement(0x00410001, "vr", value, validation_mode=config.RAISE)
+
+    @pytest.mark.parametrize("value", (bytearray(), bytearray(b"\x00\x01\x02\x03")))
+    def test_valid_o_star_bytearray(self, value):
+        for vr in ("OB", "OD", "OF", "OL", "OW", "OV"):
+            DataElement(0x00410001, "vr", value, validation_mode=config.RAISE)
+
+    @pytest.mark.parametrize("value", (-2, 4294967300))
+    def test_invalid_o_star_value(self, value):
+        for vr in ("OB", "OD", "OF", "OL", "OW", "OV"):
+            msg = f"A value of type 'int' cannot be assigned to a tag with VR {vr}"
+            with pytest.warns(UserWarning, match=msg):
+                DataElement(0x00410001, vr, value, validation_mode=config.WARN)
+            with pytest.raises(ValueError, match=msg):
+                DataElement(0x00410001, vr, value, validation_mode=config.RAISE)
+
 
 class TestBufferedDataElement:
     """Tests setting a DataElement value to a buffer"""
diff --git a/tests/test_dataset.py b/tests/test_dataset.py
index 2d5227a8c..5dca7fe3b 100644
--- a/tests/test_dataset.py
+++ b/tests/test_dataset.py
@@ -4,11 +4,12 @@
 import copy
 import io
 import math
+from pathlib import Path
 import pickle
+from platform import python_implementation
 import sys
 import weakref
 import tempfile
-from platform import python_implementation
 
 import pytest
 
@@ -2093,6 +2094,24 @@ class TestDatasetSaveAs:
         with pytest.raises(ValueError, match=msg):
             ds.save_as(DicomBytesIO())
 
+    def test_exist_ok(self):
+        """Test the exist_ok argument"""
+        ds = dcmread(get_testdata_file("MR_small.dcm"))
+        patient_name = ds.PatientName
+
+        with tempfile.TemporaryDirectory() as tdir:
+            p = Path(tdir) / "foo.dcm"
+            p.touch()
+
+            assert p.exists()
+
+            msg = r"File exists: '(.*)foo.dcm'"
+            with pytest.raises(FileExistsError, match=msg):
+                ds.save_as(p, exist_ok=False)
+
+            ds.save_as(p, exist_ok=True)
+            assert dcmread(p).PatientName == patient_name
+
 
 class TestDatasetElements:
     """Test valid assignments of data elements"""
@@ -2879,7 +2898,7 @@ class TestDatasetWithBufferedData:
                 ds = Dataset()
                 ds.PixelData = f
 
-            with pytest.raises(TypeError, match="cannot pickle"):
+            with pytest.raises(TypeError, match="cannot"):
                 pickle.dumps({"ds": ds})
 
     def test_copy_bytesio(self):
@@ -2959,7 +2978,6 @@ class TestDatasetWithBufferedData:
 
     def test_deepcopy_bufferedreader_raises(self):
         with tempfile.TemporaryDirectory() as tdir:
-            print(tdir)
             with open(f"{tdir}/foo.bin", "wb") as f:
                 f.write(b"\x00\x01\x02\x03\x04")
 
@@ -2969,7 +2987,7 @@ class TestDatasetWithBufferedData:
 
         msg = (
             r"Error deepcopying the buffered element \(7FE0,0010\) 'Pixel Data': "
-            "cannot pickle '_io.BufferedReader' object"
+            r"cannot (.*) '_io.BufferedReader' object"
         )
         with pytest.raises(TypeError, match=msg):
             copy.deepcopy(ds)
diff --git a/tests/test_fileset.py b/tests/test_fileset.py
index 439d96e89..d6ae1afd9 100644
--- a/tests/test_fileset.py
+++ b/tests/test_fileset.py
@@ -743,7 +743,7 @@ class TestRecordNode:
         item = ds.DirectoryRecordSequence[-1]
         assert "IMAGE" == item.DirectoryRecordType
         item.ReferencedFileID = "01"
-        ds.save_as(p / "DICOMDIR")
+        ds.save_as(p / "DICOMDIR", exist_ok=True)
         fs = FileSet(ds)
         assert fs._instances[0].node._file_id == Path("01")
 
diff --git a/tests/test_filewriter.py b/tests/test_filewriter.py
index ed2a7a26b..75a2ee5d6 100644
--- a/tests/test_filewriter.py
+++ b/tests/test_filewriter.py
@@ -1829,6 +1829,24 @@ class TestDCMWrite:
         for elem_orig, elem_conv in zip(ds_orig, ds_expl):
             assert elem_orig.value == elem_conv.value
 
+    def test_exist_ok(self):
+        """Test the exist_ok argument"""
+        ds = dcmread(ct_name)
+        patient_name = ds.PatientName
+
+        with tempfile.TemporaryDirectory() as tdir:
+            p = Path(tdir) / "foo.dcm"
+            p.touch()
+
+            assert p.exists()
+
+            msg = r"File exists: '(.*)foo.dcm'"
+            with pytest.raises(FileExistsError, match=msg):
+                dcmwrite(p, ds, exist_ok=False)
+
+            dcmwrite(p, ds, exist_ok=True)
+            assert dcmread(p).PatientName == patient_name
+
 
 class TestDCMWrite_EnforceFileFormat:
     """Tests for dcmwrite(enforce_file_format=True)"""