Merge pull request #316 from jacebrowning/default-pattern-args

Handle pattern arguments with default values in Manager.get()

Author: jacebrowning

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Release Notes
 
+## 2.2.1 (2024-01-04)
+
+- Updated `Manager.get()` to handle default values in pattern arguments.
+
 ## 2.2 (2023-10-14)
 
 - Added a `sync()` utility to map arbitrary objects to the filesystem.

datafiles/manager.py

@@ -21,8 +21,8 @@
 
 
 Trilean = Optional[bool]
-Missing = dataclasses._MISSING_TYPE
-_NOT_PASSED = object()  # sentinel
+Missing = dataclasses._MISSING_TYPE  # sentinel value for arguments to be loaded
+Absent = object()  # sentinel value for required arguments not passed
 
 
 class Splats:
@@ -38,31 +38,28 @@ def get(self, *args, **kwargs) -> Model:
         with hooks.disabled():
             instance = self.model.__new__(self.model)
 
-            # We **must** initialize with a value all fields on the uninitialized
-            # instance which play a role in loading, e.g., those with placeholders
-            # in the pattern. Other init fields of the instance which are not passed
-            # as args or kwargs will be set to `dataclasses._MISSING_TYPE` and their
-            # values will be loaded over.
+            # Set initial values for all passed arguments
             fields = [field for field in dataclasses.fields(self.model) if field.init]
             pattern = self.model.Meta.datafile_pattern
             args_iter = iter(args)
             for field in fields:
                 placeholder = f"{{self.{field.name}}}"
 
                 try:
-                    # we always need to consume an arg if it exists,
-                    # even if it's not one with a placeholder
                     value = next(args_iter)
                 except StopIteration:
-                    value = kwargs.get(field.name, _NOT_PASSED)
-
-                if placeholder in pattern:
-                    if value is _NOT_PASSED:
-                        raise TypeError(
-                            f"Manager.get() missing required placeholder field argument: '{field.name}'"
-                        )
-
-                if value is _NOT_PASSED:
+                    value = kwargs.get(field.name, Absent)
+
+                if (
+                    placeholder in pattern
+                    and value is Absent
+                    and isinstance(field.default, Missing)
+                ):
+                    raise TypeError(
+                        f"Manager.get() missing required placeholder field argument: '{field.name}'"
+                    )
+
+                if value is Absent:
                     if not isinstance(field.default, Missing):
                         value = field.default
                     elif not isinstance(field.default_factory, Missing):
@@ -71,7 +68,7 @@ def get(self, *args, **kwargs) -> Model:
                         value = Missing
                 object.__setattr__(instance, field.name, value)
 
-            # NOTE: the following doesn't call instance.datafile.load because hooks are disabled currently
+            # Bypass calling load() because hooks are disabled currently
             model.Model.__post_init__(instance)
 
             try:
@@ -129,7 +126,6 @@ def all(self, *, _exclude: str = "") -> Iterator[Model]:
 
         log.info(f"Finding files matching pattern: {splatted}")
         for index, filename in enumerate(iglob(splatted, recursive=True)):
-
             if Path(filename).is_dir():
                 log.debug(f"Skipped matching directory {index + 1}: {filename}")
                 continue

datafiles/tests/test_manager.py

@@ -1,4 +1,4 @@
-# pylint: disable=unused-variable
+# pylint: disable=unused-variable,unused-argument
 
 import os
 import shutil
@@ -25,35 +25,49 @@ class MyClass:
     nested: Optional[Nested] = None
 
 
+@dataclass
+class MyClassDefaults:
+    foo: int
+    bar: int = 2
+
+
 def describe_manager():
     @pytest.fixture
-    def manager():
-        shutil.rmtree(Path(__file__).parent / "files", ignore_errors=True)
+    def files():
+        return Path(__file__).parent / "files"
+
+    @pytest.fixture
+    def manager(files: Path):
+        shutil.rmtree(files, ignore_errors=True)
         model = create_model(MyClass, pattern="files/{self.foo}.yml")
         return Manager(model)
 
     @pytest.fixture
-    def manager_home():
+    def manager_at_home():
         model = create_model(Nested, pattern="~/.{self.name}.json")
         return Manager(model)
 
-    @pytest.fixture()
-    def manager_with_files():
-        files_dir = Path(__file__).parent / "files"
-        shutil.rmtree(files_dir, ignore_errors=True)
-        files_dir.mkdir(exist_ok=True)
+    @pytest.fixture
+    def manager_with_files(files: Path):
+        files.mkdir(exist_ok=True)
         model = create_model(MyClass, pattern="files/{self.foo}.yml")
         model(foo=1, bar=2).datafile.save()
         return Manager(model)
 
+    @pytest.fixture
+    def manager_with_defaults(files: Path):
+        shutil.rmtree(files, ignore_errors=True)
+        model = create_model(MyClassDefaults, pattern="files/{self.foo}/{self.bar}.yml")
+        return Manager(model)
+
     def describe_get():
         @patch("datafiles.mapper.Mapper.load")
         @patch("datafiles.mapper.Mapper.exists", True)
         @patch("datafiles.mapper.Mapper.modified", False)
-        def when_partial_args_passed_init_args_missing(mock_load, expect, manager):
-            got = manager.get(1)
-            expect(got.foo) == 1
-            expect(got.bar) is Missing
+        def when_absent_pattern_arg(mock_load, expect, manager):
+            instance = manager.get(1)
+            expect(instance.foo) == 1
+            expect(instance.bar) is Missing
             expect(mock_load.called).is_(True)
 
             with expect.raises(
@@ -62,35 +76,39 @@ def when_partial_args_passed_init_args_missing(mock_load, expect, manager):
             ):
                 manager.get(bar=2)
 
+        @patch("datafiles.mapper.Mapper.load")
         @patch("datafiles.mapper.Mapper.exists", True)
         @patch("datafiles.mapper.Mapper.modified", False)
-        def when_partial_args_passed_init_arg_missing_file_exists(
-            expect, manager_with_files
+        def when_absent_pattern_arg_has_default_value(
+            mock_load, expect, manager_with_defaults: Manager
         ):
-            # demonstrates that `Manager.get` loads the value for bar, when it is not passed
+            expect(manager_with_defaults.get(0, 1)) == MyClassDefaults(foo=0, bar=1)
+            expect(manager_with_defaults.get(1)) == MyClassDefaults(foo=1, bar=2)
+            expect(mock_load.called).is_(True)
+
+        @patch("datafiles.mapper.Mapper.exists", True)
+        @patch("datafiles.mapper.Mapper.modified", False)
+        def when_partial_args_match_file(expect, manager_with_files: Manager):
             expect(manager_with_files.get(1)) == MyClass(foo=1, bar=2)
 
         @patch("datafiles.mapper.Mapper.exists", True)
         @patch("datafiles.mapper.Mapper.modified", False)
-        def when_partial_kwargs_passed_init_arg_missing_file_exists(
-            expect, manager_with_files
-        ):
-            # demonstrates that `Manager.get` loads the value for bar, when it is not passed
+        def when_partial_kwargs_match_file(expect, manager_with_files: Manager):
             expect(manager_with_files.get(foo=1)) == MyClass(foo=1, bar=2)
 
     def describe_get_or_none():
         @patch("datafiles.mapper.Mapper.load")
         @patch("datafiles.mapper.Mapper.exists", True)
         @patch("datafiles.mapper.Mapper.modified", False)
-        def when_file_exists(mock_load, expect, manager):
+        def when_file_exists(mock_load, expect, manager: Manager):
             expect(manager.get_or_none(foo=1, bar=2)) == MyClass(foo=1, bar=2)
             expect(mock_load.called).is_(True)
 
         @patch("datafiles.mapper.Mapper.exists", False)
-        def when_file_missing(expect, manager):
+        def when_file_missing(expect, manager: Manager):
             expect(manager.get_or_none(foo=3, bar=4)).is_(None)
 
-        def when_file_corrupt(expect, manager):
+        def when_file_corrupt(expect, manager: Manager):
             instance = manager.get_or_create(foo=2, bar=1)
             instance.datafile.path.write_text("{")
             instance2 = manager.get_or_none(foo=2, bar=2)
@@ -102,48 +120,48 @@ def describe_get_or_create():
         @patch("datafiles.mapper.Mapper.load")
         @patch("datafiles.mapper.Mapper.exists", True)
         @patch("datafiles.mapper.Mapper.modified", False)
-        def when_file_exists(mock_save, mock_load, expect, manager):
+        def when_file_exists(mock_save, mock_load, expect, manager: Manager):
             expect(manager.get_or_create(foo=1, bar=2)) == MyClass(foo=1, bar=2)
             expect(mock_save.called).is_(True)
             expect(mock_load.called).is_(False)
 
         @patch("datafiles.mapper.Mapper.save")
         @patch("datafiles.mapper.Mapper.load")
         @patch("datafiles.mapper.Mapper.exists", False)
-        def when_file_missing(mock_save, mock_load, expect, manager):
+        def when_file_missing(mock_save, mock_load, expect, manager: Manager):
             expect(manager.get_or_create(foo=1, bar=2)) == MyClass(foo=1, bar=2)
             expect(mock_save.called).is_(True)
             expect(mock_load.called).is_(True)
 
-        def when_file_corrupt(expect, manager):
+        def when_file_corrupt(expect, manager: Manager):
             instance = manager.get_or_create(foo=2, bar=1)
             instance.datafile.path.write_text("{")
             instance2 = manager.get_or_create(foo=2, bar=2)
-            expect(instance2.bar) == 2
+            expect(instance2.bar) == 2  # type: ignore[attr-defined]
 
     def describe_all():
         @patch("datafiles.mapper.Mapper.exists", False)
-        def when_no_files_exist(expect, manager):
+        def when_no_files_exist(expect, manager: Manager):
             items = list(manager.all())
             expect(items) == []
 
-        def with_home_directory(expect, manager_home):
-            items = list(manager_home.all())
+        def with_home_directory(expect, manager_at_home: Manager):
+            items = list(manager_at_home.all())
             if "CI" not in os.environ:
                 expect(len(items)) > 0
 
     def describe_filter():
         @patch("datafiles.mapper.Mapper.exists", False)
-        def when_no_files_exist(expect, manager):
+        def when_no_files_exist(expect, manager: Manager):
             items = list(manager.filter())
             expect(items) == []
 
         @patch("datafiles.mapper.Mapper.exists", False)
-        def with_partial_positional_arguments(expect, manager):
+        def with_partial_positional_arguments(expect, manager: Manager):
             items = list(manager.filter(foo=1))
             expect(items) == []
 
         @patch("datafiles.mapper.Mapper.exists", False)
-        def with_nested_key_query(expect, manager):
+        def with_nested_key_query(expect, manager: Manager):
             items = list(manager.filter(nested__name="John Doe"))
             expect(items) == []

pyproject.toml

@@ -1,7 +1,7 @@
 [tool.poetry]
 
 name = "datafiles"
-version = "2.2"
+version = "2.2.1"
 description = "File-based ORM for dataclasses."
 
 license = "MIT"