From 4487710680e79323c85c61b1a352af9285532372 Mon Sep 17 00:00:00 2001 From: Nathan McDougall Date: Tue, 19 May 2026 14:31:03 +1200 Subject: [PATCH 1/5] feat: add Document.get() and Editor.get() for non-raising value access --- doc/specs/2026-05-19-get-method-design.md | 94 +++++++++++++++++++++++ src/yamltrip/document.py | 8 ++ src/yamltrip/editor.py | 4 + tests/test_document.py | 47 ++++++++++++ tests/test_editor.py | 19 +++++ 5 files changed, 172 insertions(+) create mode 100644 doc/specs/2026-05-19-get-method-design.md diff --git a/doc/specs/2026-05-19-get-method-design.md b/doc/specs/2026-05-19-get-method-design.md new file mode 100644 index 0000000..d4a0b12 --- /dev/null +++ b/doc/specs/2026-05-19-get-method-design.md @@ -0,0 +1,94 @@ +# Document.get() — Non-Raising Value Access + +**Date:** 2026-05-19 +**Issue:** #23 + +## Problem + +Accessing `doc.root` or `doc[()]` on a document with no YAML value (empty string, comment-only, or `---` only) raises `QueryError`. This forces callers to wrap every access in try/except even though an empty document is a normal state. + +The request proposes making `.root` return `None`, but that conflates "document has no value" with "document explicitly contains null" — two semantically different states. A `.get()` pattern keeps the distinction clear and gives callers control over the fallback. + +## Design + +Add a `get(*keys, default=None)` method to `Document` and `Editor` that returns the parsed Python value at the given path, or `default` if the path doesn't exist. + +### Signature + +```python +def get(self, *keys: KeyPart, default: Any = None) -> Any: +``` + +### Semantics + +| Expression | Result | +|------------|--------| +| `doc.get()` | Root value, or `None` if empty doc | +| `doc.get('tool', 'ruff')` | Value at path, or `None` if missing | +| `doc.get('missing', default={})` | `{}` | +| `doc.get('existing_null_key')` | `None` (the actual YAML null) | +| `doc.root` | Unchanged — still raises `QueryError` on empty docs | + +### Null Ambiguity + +`doc.get('key')` returns `None` both for "key missing" and "key has YAML null value". Callers who need to distinguish use `'key' in doc` first. This is the same trade-off as `dict.get()`. + +## Implementation + +### Document.get() + +```python +def get(self, *keys: KeyPart, default: Any = None) -> Any: + """Return the parsed value at path, or default if the path doesn't exist.""" + normalized = _normalize_keys(keys) if keys else () + route = _make_route(normalized) + if not self._core_doc.query_exists(route): + return default + return self._core_doc.parse_value(route) +``` + +Uses `query_exists` (the same mechanism as `__contains__`) to check existence before accessing. No exception handling in the happy path. + +### Editor.get() + +Delegates to the underlying document: + +```python +def get(self, *keys: KeyPart, default: Any = None) -> Any: + """Return the parsed value at path, or default if missing.""" + return self._document.get(*keys, default=default) +``` + +## Change Locations + +- `src/yamltrip/document.py` — add `get()` method to `Document` +- `src/yamltrip/editor.py` — add `get()` method to `Editor` +- No Rust changes required + +## Testing + +New tests: + +- `doc.get()` on empty document → `None` +- `doc.get()` on comment-only document → `None` +- `doc.get()` on `---\n` document → `None` +- `doc.get()` on document with root value → returns the value +- `doc.get('key')` on existing key → returns value +- `doc.get('key')` on missing key → `None` +- `doc.get('key', default={})` on missing key → `{}` +- `doc.get('nested', 'path')` on existing nested path → value +- `doc.get('nested', 'missing')` on partial path → `None` +- `doc.get('null_key')` where value is YAML null → `None` +- `doc.root` still raises on empty docs (no regression) +- `Editor.get()` mirrors Document behavior + +## Scope Boundaries + +**In scope:** +- `Document.get()` method +- `Editor.get()` method + +**Out of scope:** +- Changing `.root` behavior +- Adding `.get()` to the stub file (`_core.pyi`) — this is Python-only +- Sentinel-based missing detection (use `in` operator for that) diff --git a/src/yamltrip/document.py b/src/yamltrip/document.py index 045dac6..90bed64 100644 --- a/src/yamltrip/document.py +++ b/src/yamltrip/document.py @@ -98,6 +98,14 @@ def root(self) -> Any: """The entire document parsed as a Python object.""" return self[()] + def get(self, *keys: KeyPart, default: Any = None) -> Any: + """Return the parsed value at path, or default if the path doesn't exist.""" + normalized = _normalize_keys(keys) if keys else () + route = _make_route(normalized) + if not self._core_doc.query_exists(route): + return default + return self._core_doc.parse_value(route) + def __eq__(self, other: object) -> bool: """Compare documents by their source text.""" if not isinstance(other, Document): diff --git a/src/yamltrip/editor.py b/src/yamltrip/editor.py index 898755e..5a3d9b0 100644 --- a/src/yamltrip/editor.py +++ b/src/yamltrip/editor.py @@ -88,6 +88,10 @@ def root(self) -> Any: """The entire document parsed as a Python object.""" return self.document.root + def get(self, *keys: KeyPart, default: Any = None) -> Any: + """Return the parsed value at path, or default if missing.""" + return self.document.get(*keys, default=default) + def __getitem__(self, keys: object) -> Any: """Retrieve the parsed value at the given path.""" return self.document[keys] diff --git a/tests/test_document.py b/tests/test_document.py index 5afa49b..438ed5d 100644 --- a/tests/test_document.py +++ b/tests/test_document.py @@ -143,6 +143,53 @@ def test_nested_key_missing(self): assert ("a", "c") not in doc +class TestDocumentGet: + def test_empty_document_returns_default(self): + doc = Document("") + assert doc.get() is None + + def test_comment_only_returns_default(self): + doc = Document("# just a comment\n") + assert doc.get() is None + + def test_directive_only_returns_default(self): + doc = Document("---\n") + assert doc.get() is None + + def test_root_value_returned(self): + doc = Document("name: foo\n") + assert doc.get() == {"name": "foo"} + + def test_existing_key(self): + doc = Document("name: foo\nversion: 1\n") + assert doc.get("name") == "foo" + + def test_missing_key_returns_none(self): + doc = Document("name: foo\n") + assert doc.get("missing") is None + + def test_missing_key_returns_custom_default(self): + doc = Document("name: foo\n") + assert doc.get("missing", default={}) == {} + + def test_nested_existing_path(self): + doc = Document("a:\n b: 1\n") + assert doc.get("a", "b") == 1 + + def test_nested_missing_path(self): + doc = Document("a:\n b: 1\n") + assert doc.get("a", "c") is None + + def test_null_value_returns_none(self): + doc = Document("key: null\n") + assert doc.get("key") is None + + def test_root_still_raises_on_empty(self): + doc = Document("") + with pytest.raises(QueryError): + _ = doc.root + + class TestDocumentInspection: def test_query_returns_feature(self): doc = Document("name: foo") diff --git a/tests/test_editor.py b/tests/test_editor.py index 5bd9e86..0cc3e0c 100644 --- a/tests/test_editor.py +++ b/tests/test_editor.py @@ -137,3 +137,22 @@ def test_document_outside_context(self, yaml_file): def test_repr(self, yaml_file): editor = Editor(yaml_file) assert "Editor(" in repr(editor) + + +class TestEditorGet: + def test_get_existing_key(self, yaml_file): + with Editor(yaml_file) as editor: + assert editor.get("name") == "foo" + + def test_get_missing_key(self, yaml_file): + with Editor(yaml_file) as editor: + assert editor.get("missing") is None + + def test_get_missing_key_with_default(self, yaml_file): + with Editor(yaml_file) as editor: + assert editor.get("missing", default="fallback") == "fallback" + + def test_get_root(self, yaml_file): + with Editor(yaml_file) as editor: + result = editor.get() + assert result["name"] == "foo" From 3bb4fc33895179fcf3e0c00cc4ebd72f2b783037 Mon Sep 17 00:00:00 2001 From: Nathan McDougall Date: Tue, 19 May 2026 14:32:12 +1200 Subject: [PATCH 2/5] chore: disable B018 (useless expression) in test suite --- ruff.toml | 2 +- tests/test_document.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ruff.toml b/ruff.toml index 308c5e7..c2a8cc2 100644 --- a/ruff.toml +++ b/ruff.toml @@ -32,7 +32,7 @@ lint.select = [ ] lint.ignore = [ "PLR2004", "S101", "SIM108" ] lint.per-file-ignores."!tests/**/*.py" = [ "ARG002", "PT" ] -lint.per-file-ignores."tests/**" = [ "D", "INP", "S603", "TC" ] +lint.per-file-ignores."tests/**" = [ "B018", "D", "INP", "S603", "TC" ] lint.flake8-builtins.strict-checking = true lint.flake8-type-checking.quote-annotations = true lint.flake8-type-checking.strict = true diff --git a/tests/test_document.py b/tests/test_document.py index 438ed5d..55a3839 100644 --- a/tests/test_document.py +++ b/tests/test_document.py @@ -187,7 +187,7 @@ def test_null_value_returns_none(self): def test_root_still_raises_on_empty(self): doc = Document("") with pytest.raises(QueryError): - _ = doc.root + doc.root class TestDocumentInspection: From 476b503cc589b0ac95fa393bd9a0d0c1c6627619 Mon Sep 17 00:00:00 2001 From: Nathan McDougall Date: Tue, 19 May 2026 15:20:32 +1200 Subject: [PATCH 3/5] refactor: address review feedback on Document.get() - Switch from query_exists+parse_value to try/except KeyError pattern - Remove redundant ternary; always call _normalize_keys(keys) --- src/yamltrip/document.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/yamltrip/document.py b/src/yamltrip/document.py index 90bed64..4b1611b 100644 --- a/src/yamltrip/document.py +++ b/src/yamltrip/document.py @@ -100,11 +100,12 @@ def root(self) -> Any: def get(self, *keys: KeyPart, default: Any = None) -> Any: """Return the parsed value at path, or default if the path doesn't exist.""" - normalized = _normalize_keys(keys) if keys else () + normalized = _normalize_keys(keys) route = _make_route(normalized) - if not self._core_doc.query_exists(route): + try: + return self._core_doc.parse_value(route) + except KeyError: return default - return self._core_doc.parse_value(route) def __eq__(self, other: object) -> bool: """Compare documents by their source text.""" From ab5a6b0fc4a0230a2fef060118e81ba31663e07f Mon Sep 17 00:00:00 2001 From: Nathan McDougall Date: Wed, 20 May 2026 08:03:01 +1200 Subject: [PATCH 4/5] docs: remove proposed implementations from get-method spec --- doc/specs/2026-05-19-get-method-design.md | 26 ----------------------- 1 file changed, 26 deletions(-) diff --git a/doc/specs/2026-05-19-get-method-design.md b/doc/specs/2026-05-19-get-method-design.md index d4a0b12..b8e060d 100644 --- a/doc/specs/2026-05-19-get-method-design.md +++ b/doc/specs/2026-05-19-get-method-design.md @@ -33,32 +33,6 @@ def get(self, *keys: KeyPart, default: Any = None) -> Any: `doc.get('key')` returns `None` both for "key missing" and "key has YAML null value". Callers who need to distinguish use `'key' in doc` first. This is the same trade-off as `dict.get()`. -## Implementation - -### Document.get() - -```python -def get(self, *keys: KeyPart, default: Any = None) -> Any: - """Return the parsed value at path, or default if the path doesn't exist.""" - normalized = _normalize_keys(keys) if keys else () - route = _make_route(normalized) - if not self._core_doc.query_exists(route): - return default - return self._core_doc.parse_value(route) -``` - -Uses `query_exists` (the same mechanism as `__contains__`) to check existence before accessing. No exception handling in the happy path. - -### Editor.get() - -Delegates to the underlying document: - -```python -def get(self, *keys: KeyPart, default: Any = None) -> Any: - """Return the parsed value at path, or default if missing.""" - return self._document.get(*keys, default=default) -``` - ## Change Locations - `src/yamltrip/document.py` — add `get()` method to `Document` From de13834de9af90aa3cd7cdd751d94fd60107d1b4 Mon Sep 17 00:00:00 2001 From: Nathan McDougall Date: Wed, 20 May 2026 08:10:15 +1200 Subject: [PATCH 5/5] fix: translate ValueError to QueryError in Document.get() --- src/yamltrip/document.py | 2 ++ tests/test_document.py | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/src/yamltrip/document.py b/src/yamltrip/document.py index 4b1611b..f37e716 100644 --- a/src/yamltrip/document.py +++ b/src/yamltrip/document.py @@ -106,6 +106,8 @@ def get(self, *keys: KeyPart, default: Any = None) -> Any: return self._core_doc.parse_value(route) except KeyError: return default + except ValueError as e: + raise QueryError(str(e)) from None def __eq__(self, other: object) -> bool: """Compare documents by their source text.""" diff --git a/tests/test_document.py b/tests/test_document.py index 55a3839..99dcc8b 100644 --- a/tests/test_document.py +++ b/tests/test_document.py @@ -184,6 +184,12 @@ def test_null_value_returns_none(self): doc = Document("key: null\n") assert doc.get("key") is None + def test_malformed_value_raises_query_error(self): + """get() should raise QueryError (not ValueError) for unparsable values.""" + doc = Document("a:\n b: 1\n b: 2") + with pytest.raises(QueryError): + doc.get("a") + def test_root_still_raises_on_empty(self): doc = Document("") with pytest.raises(QueryError):