Conversation
- Introduce a new `convert.parse` recipe wrangle that parses JSON, Python literals, and YAML-like object text into JSON-compatible Python values. The implementation adds expected-type validation, per-column defaults, missing-value handling, normalization for numpy-backed objects, and safer YAML scalar resolution. - Eventually could replace from_yaml, from_json - Tests cover valid inputs, quoted structures, defaults, type mismatches, invalid values, and multi-column behavior.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Default normalization, schema validation, and YAML alias resource-safety issues remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds convert.parse for converting JSON, Python literals, and YAML-like text into JSON-compatible Python values.
Changes:
- Adds parsing, normalization, expected-type validation, and fallback handling.
- Adds comprehensive recipe-level tests for supported inputs and edge cases.
File summaries
| File | Description |
|---|---|
wrangles/recipe_wrangles/convert.py |
Implements convert.parse and its schema. |
tests/recipes/wrangles/test_convert.py |
Tests parsing, defaults, validation, and multiple columns. |
Review details
Suppressed comments (1)
wrangles/recipe_wrangles/convert.py:645
- The invalid/type-mismatch fallback also skips
_normalize_json_compatible, so NumPy-backed or otherwise unsupported defaults can escape unchanged. Apply the same normalization as successful parsed values so every return path honors the JSON-compatible result contract.
return _copy.deepcopy(col_default)
- Files reviewed: 2/2 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| pass | ||
|
|
||
| try: | ||
| return _yaml.load(value, Loader=_ObjectYAMLLoader) |
Comment on lines
+579
to
+592
| type: | ||
| - string | ||
| - array | ||
| description: >- | ||
| Required result category. Use any, dictionary, list, or scalar. | ||
| A list may provide one category per input column. | ||
| default: any | ||
| items: | ||
| type: string | ||
| enum: | ||
| - any | ||
| - dictionary | ||
| - list | ||
| - scalar |
| if _is_missing_object_value(value): | ||
| if col_default is _DEFAULT_NOT_SET: | ||
| return None | ||
| return _copy.deepcopy(col_default) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Introduce a new
convert.parserecipe wrangle that parses JSON, Python literals, and YAML-like object text into JSON-compatible Python values. The implementation adds expected-type validation, per-column defaults, missing-value handling, normalization for numpy-backed objects, and safer YAML scalar resolution.Eventually could replace from_yaml, from_json
Tests cover valid inputs, quoted structures, defaults, type mismatches, invalid values, and multi-column behavior.
Linked issue
Closes #
What changes
Describe the user-visible or API behavior change. Keep unrelated changes in a
separate PR.
How it was verified
List the focused tests and any manual or live-service verification performed.
Compatibility and risk
Describe compatibility impact, migration needs, credentials/external services,
and the safest rollback.
Ready-for-review checklist
mainand has no merge conflictsSee the pull request workflow.