From 5a4c82a4245a82d86ee98ca11286f04a0e262faf Mon Sep 17 00:00:00 2001
From: Alexandre Gramfort <alexandre.gramfort@m4x.org>
Date: Mon, 31 Jan 2022 09:54:22 +0100
Subject: [PATCH] FIX: make update unalter the path instance if check fails
 (#950)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* FIX: make update unalter the path instance if check fails

* fix

* update what's new

Co-authored-by: Richard Höchenberger <richard.hoechenberger@gmail.com>
---
 doc/whats_new.rst           |  2 ++
 mne_bids/path.py            | 14 ++++++++++++--
 mne_bids/tests/test_path.py |  9 +++++++++
 3 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/doc/whats_new.rst b/doc/whats_new.rst
index d48a20aa3..bce2430cc 100644
--- a/doc/whats_new.rst
+++ b/doc/whats_new.rst
@@ -63,6 +63,8 @@ Bug fixes
 
 - Avoid ``DeprecationWarning`` in :func:`mne_bids.inspect_dataset` with the upcoming MNE-Python 1.0 release, by `Richard Höchenberger`_ (:gh:`942`)
 
+- Avoid modifying the instance of :class:`mne_bids.BIDSPath` if validation fails when calling :meth:`mne_bids.BIDSPath.update`, by `Alexandre Gramfort`_ (:gh:`950`)
+
 :doc:`Find out what was new in previous releases <whats_new_previous_releases>`
 
 .. include:: authors.rst
diff --git a/mne_bids/path.py b/mne_bids/path.py
index 7d2166f6b..8325cb78b 100644
--- a/mne_bids/path.py
+++ b/mne_bids/path.py
@@ -714,6 +714,7 @@ def update(self, *, check=None, **kwargs):
                 kwargs['extension'] = f'.{extension}'
 
         # error check entities
+        old_kwargs = dict()
         for key, val in kwargs.items():
 
             # check if there are any characters not allowed
@@ -728,6 +729,8 @@ def update(self, *, check=None, **kwargs):
             # set entity value, ensuring `root` is a Path
             if val is not None and key == 'root':
                 val = Path(val).expanduser()
+            old_kwargs[key] = \
+                getattr(self, f'{key}') if hasattr(self, f'_{key}') else None
             setattr(self, f'_{key}', val)
 
         # infer datatype if suffix is uniquely the datatype
@@ -735,8 +738,15 @@ def update(self, *, check=None, **kwargs):
                 self.suffix in SUFFIX_TO_DATATYPE:
             self._datatype = SUFFIX_TO_DATATYPE[self.suffix]
 
-        # Perform a check of the entities.
-        self._check()
+        # Perform a check of the entities and revert changes if check fails
+        try:
+            self._check()
+        except Exception as e:
+            old_check = self.check
+            self.check = False
+            self.update(**old_kwargs)
+            self.check = old_check
+            raise e
         return self
 
     def match(self, check=False):
diff --git a/mne_bids/tests/test_path.py b/mne_bids/tests/test_path.py
index 23f769b42..81fce0620 100644
--- a/mne_bids/tests/test_path.py
+++ b/mne_bids/tests/test_path.py
@@ -1134,3 +1134,12 @@ def test_datasetdescription_with_bidspath(return_bids_test_dir):
     bids_path.update(suffix='dataset_description', check=False)
     assert bids_path.fpath.as_posix() == \
         Path(f'{return_bids_test_dir}/dataset_description.json').as_posix()
+
+
+def test_update_fail_check_no_change():
+    bids_path = BIDSPath(subject='test')
+    try:
+        bids_path.update(suffix='ave')
+    except Exception:
+        pass
+    assert bids_path.suffix is None