Fix recursion errors in Ascleandict and validate dataclass exports#16
Fix recursion errors in Ascleandict and validate dataclass exports#16MrKevinWeiss merged 2 commits intomasterfrom
Conversation
There were recursion errors... possibly from the user side but it would be good to fix them.
There was a problem hiding this comment.
Pull request overview
This PR hardens LobHlpr.ascleandict() against infinite recursion by detecting circular references during dataclass/container conversion, and adds regression tests to ensure circular list/dict inputs are handled deterministically.
Changes:
- Add circular-reference detection to the internal
_converttraversal using an_seenobject-id set and a placeholder sentinel string. - Emit a warning when a circular reference is encountered instead of recursing indefinitely.
- Add tests covering circular self-references in a list and a dict.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/lob_hlpr/hlpr.py |
Track seen container objects during ascleandict conversion to prevent circular-recursion crashes and return a sentinel placeholder. |
tests/test_lob_hlpr.py |
Add regression tests asserting circular list/dict references are converted to sentinel placeholders without crashing. |
| logging.getLogger(__name__).warning( | ||
| "ascleandict: circular reference detected in %s, skipping", | ||
| type(obj).__name__, | ||
| ) | ||
| return f"<circular ref: {type(obj).__name__}>" |
There was a problem hiding this comment.
The warning message says the circular reference is being "skipped", but the function actually returns a placeholder string ("<circular ref: …>") that is kept in the output. Consider adjusting the log message to reflect the real behavior (e.g., “replacing with placeholder”) so logs aren’t misleading. Also consider using a module-level logger instead of calling logging.getLogger(name) on every circular detection.
| def _convert(obj: object, _seen: set | None = None) -> Any: | ||
| is_container = isinstance(obj, (dict, list)) or ( | ||
| is_dataclass(obj) and not isinstance(obj, type) | ||
| ) | ||
| if is_container: |
There was a problem hiding this comment.
The PR title mentions "validate dataclass exports", but the diff only changes ascleandict recursion handling and adds circular-reference tests. If export validation is intended, it looks missing; otherwise please update the PR title/description to match the actual changes to avoid confusion in release notes/history.
This pull request improves the robustness of the
ascleandictutility insrc/lob_hlpr/hlpr.pyby adding detection and handling of circular references in dataclasses, lists, and dictionaries. It also adds new tests to ensure that these cases are handled gracefully without causing crashes or infinite recursion.Handling of circular references:
_convertfunction to track seen objects using their IDs, detect circular references in containers (dataclasses, lists, dicts), and replace them with a sentinel string (e.g.,"<circular ref: list>"). This prevents infinite recursion and logs a warning when a circular reference is detected. [1] [2] [3]Testing:
test_ascleandict_circular_reference_in_listandtest_ascleandict_circular_reference_in_dictto verify that circular references in lists and dicts are handled as expected and do not cause crashes.