Reject empty keypaths separator#580
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #580 +/- ##
=======================================
Coverage 98.32% 98.32%
=======================================
Files 64 64
Lines 2392 2394 +2
=======================================
+ Hits 2352 2354 +2
Misses 40 40
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR tightens validation in benedict.core.keypaths.keypaths() by treating an explicit empty separator="" as invalid input (raising ValueError) rather than silently defaulting to ".", aligning runtime behavior with the documented contract.
Changes:
- Change separator defaulting logic to only fall back to
"."whenseparator is None(not when it is an empty string). - Add validation to reject empty-string separators with a
ValueError. - Add a unit test asserting
separator=""raisesValueError.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
benedict/core/keypaths.py |
Makes separator defaulting explicit (None only) and raises on empty separators. |
tests/core/test_keypaths.py |
Adds coverage for rejecting an empty separator. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| separator = "." if separator is None else separator | ||
| if not type_util.is_string(separator): | ||
| raise ValueError("separator argument must be a (non-empty) string.") | ||
| if not separator: | ||
| raise ValueError("separator argument must be a (non-empty) string.") |
| def test_keypaths_with_empty_separator(self) -> None: | ||
| with self.assertRaises(ValueError): | ||
| _ = _keypaths({"a": {"b": 1}}, separator="") | ||
|
|
What changed
keypaths(..., separator="")now raises the sameValueErroras other invalid separators instead of silently falling back to the default"."separator.Why
The function documents and reports that the separator must be a non-empty string. An explicit empty separator is most likely a caller mistake, and silently converting it to
"."can hide the invalid input and return keypaths with an unexpected separator.separator=Nonestill preserves the previous default behavior.Validation
python -m pytest tests\core\test_keypaths.py -qpython -m pytest tests\core -qpython -m ruff check benedict\core\keypaths.py tests\core\test_keypaths.pypython -m black --check benedict\core\keypaths.py tests\core\test_keypaths.pygit diff --check