Skip to content

feat: add Document.get() and Editor.get() for non-raising value access#27

Merged
nathanjmcdougall merged 5 commits into
mainfrom
23-root-property-should-return-none-on-empty-documents-instead-of-raising-queryerror
May 19, 2026
Merged

feat: add Document.get() and Editor.get() for non-raising value access#27
nathanjmcdougall merged 5 commits into
mainfrom
23-root-property-should-return-none-on-empty-documents-instead-of-raising-queryerror

Conversation

@nathanjmcdougall
Copy link
Copy Markdown
Collaborator

Closes #23

Summary

Adds a .get(*keys, default=None) method to Document and Editor that returns the parsed Python value at a path, or default if the path doesn't exist.

This provides a dict.get()-style alternative to bracket access without changing the existing strict behavior of .root and __getitem__.

Design decision

The issue requested making .root return None on empty documents. We chose .get() instead because:

  • Returning None from .root conflates "no value" with "explicit YAML null" — two distinct states
  • .get(default=...) gives callers control over the fallback
  • The pattern is immediately familiar to Python developers

Examples

doc = yamltrip.loads('')
doc.get()                        # None (empty doc)
doc.get(default={})              # {}

doc = yamltrip.loads('name: foo\n')
doc.get('name')                  # 'foo'
doc.get('missing')               # None
doc.get('missing', default='x')  # 'x'

Changes

  • src/yamltrip/document.pyDocument.get() method
  • src/yamltrip/editor.pyEditor.get() method
  • ruff.toml — disable B018 in tests (property access for side-effects is legitimate in tests)
  • Tests covering empty docs, missing keys, nested paths, null values, custom defaults

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a dict.get()-style accessor to Document and Editor so callers can fetch parsed values without handling QueryError for missing paths or empty documents. The existing strict .root / __getitem__ semantics are preserved, and the design rationale (avoid conflating "no value" with explicit YAML null) is documented in a new spec file.

Changes:

  • New Document.get(*keys, default=None) using query_exists + parse_value, and a thin Editor.get() delegate.
  • Tests for empty / comment-only / --- docs, nested paths, missing keys, custom defaults, YAML null, and a regression test confirming .root still raises.
  • Ruff config tweak to ignore B018 in tests (allowing bare attribute access for side-effect assertions like doc.root), plus a design spec doc.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/yamltrip/document.py Adds Document.get() returning default when the path does not exist.
src/yamltrip/editor.py Adds Editor.get() delegating to the underlying document.
tests/test_document.py New TestDocumentGet covering empty docs, nested paths, defaults, null values, .root regression.
tests/test_editor.py New TestEditorGet mirroring document behavior through the editor context.
ruff.toml Enables B018 ignore under tests/** to permit property-access-for-side-effects assertions.
doc/specs/2026-05-19-get-method-design.md New design spec describing the rationale, semantics, and scope of .get().

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/yamltrip/document.py Outdated
Comment thread src/yamltrip/document.py Outdated
Comment thread tests/test_document.py
- Switch from query_exists+parse_value to try/except KeyError pattern
- Remove redundant ternary; always call _normalize_keys(keys)
Copy link
Copy Markdown
Collaborator Author

@nathanjmcdougall nathanjmcdougall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed all three review comments in 476b503:

Comment 1 (double traversal): Agreed — switched to try/except KeyError pattern. ValueError from malformed nodes still propagates correctly since we only catch KeyError.

Comment 2 (type validation ternary): The premise was slightly off — _normalize_keys was already being called for non-empty keys (e.g. doc.get(1.5) → keys=(1.5,) → truthy → validation fires). However, agreed on simplifying: removed the redundant ternary since _normalize_keys(()) handles the empty case correctly.

Comment 3 (test for invalid key types): The type checker ( y) already catches these at static analysis time, making runtime tests redundant. No additional tests needed.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Comment thread src/yamltrip/document.py
Comment thread doc/specs/2026-05-19-get-method-design.md Outdated
@nathanjmcdougall nathanjmcdougall merged commit 9e4b1c0 into main May 19, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

root property should return None on empty documents instead of raising QueryError

2 participants