Skip to content

convert.from_json and convert.from_yaml accept default as a list when… - #987

Merged
ebhills merged 8 commits into
mainfrom
985-enhancement-allow-list-of-onemultiple-defaults
Jul 26, 2026
Merged

convert.from_json and convert.from_yaml accept default as a list when…#987
ebhills merged 8 commits into
mainfrom
985-enhancement-allow-list-of-onemultiple-defaults

Conversation

@mborodii-prog

@mborodii-prog mborodii-prog commented May 18, 2026

Copy link
Copy Markdown
Contributor

Summary

convert.from_json and convert.from_yaml now accept a list of defaults when input is also a list of columns. Previously, passing default as a YAML list (e.g. default: - {}) caused an error because only scalar defaults were supported.

Behaviour

default value input columns What happens
Any scalar ({}, null, 0, …) any Broadcast to all columns (unchanged)
[] — empty list any [] used as the default for every column
[value] — 1-element list any Unwrap value and broadcast to all columns
[v1, v2, …] — n-element list n columns Each column gets the corresponding default
[v1, v2, v3] — n-element list m ≠ n columns ValueError (length mismatch)

Examples

1 — Original issue: single column with a list-style default

wrangles:
  - convert.from_json:
      input:
        - Dict
      default:
        - {}        # {} applied to the one input column
      output:
        - dict
Dict (input) dict (output)
'{"a": 1}' {"a": 1}
'' {}
'bad json' {}

2 — One default broadcast across multiple columns

wrangles:
  - convert.from_json:
      input:
        - col1
        - col2
      default:
        - {}        # {} applied to both columns
      output:
        - out1
        - out2
col1 col2 out1 out2
'{"a":1}' '{"b":2}' {"a": 1} {"b": 2}
'' '' {} {}

3 — Different default per column

wrangles:
  - convert.from_json:
      input:
        - col1
        - col2
      default:
        - {}        # {} for col1
        - []        # [] for col2
      output:
        - out1
        - out2
col1 col2 out1 out2
'{"a":1}' '[1,2,3]' {"a": 1} [1, 2, 3]
'' '' {} []

4 — Same patterns work for convert.from_yaml

wrangles:
  - convert.from_yaml:
      input:
        - col1
        - col2
      default:
        - {}
        - []
      output:
        - out1
        - out2

5 — Empty list as a default value (pre-existing behaviour preserved)

wrangles:
  - convert.from_json:
      input: header1
      default: []   # scalar empty-list default — unchanged behaviour
header1 header1 (output)
'[1,2,3]' [1, 2, 3]
'' []

6 — Length mismatch raises an error

wrangles:
  - convert.from_json:
      input:
        - col1
        - col2
      default:
        - {}
        - []
        - null      # 3 defaults for 2 columns → ValueError
      output:
        - out1
        - out2
ValueError: The list of default values must be the same length as input/output for convert.from_json

Implementation notes

  • The guard condition is isinstance(default, list) and len(default) > 0. An empty list ([]) is unambiguously a default value, not a zero-length per-column list, so it falls through to the scalar broadcast path.
  • A single-element list ([value]) is always unwrapped and broadcast regardless of how many input columns there are.

@mborodii-prog mborodii-prog linked an issue May 18, 2026 that may be closed by this pull request
@mborodii-prog
mborodii-prog marked this pull request as draft May 18, 2026 14:26
@mborodii-prog
mborodii-prog marked this pull request as ready for review May 19, 2026 07:26
@ebhills

ebhills commented May 19, 2026

Copy link
Copy Markdown
Collaborator

@mborodii-prog : this 3rd behavior is confusing / too 'magical':

[1, 2, 3] (list, len ≠ col count) 2 [1, 2, 3] used as-is for every column

I would recommend we stick to 2 behaviors only:

  • same number of outputs and defaults -> map them to each other
  • only one default -> applies to all.

This will meet the typical use cases and is easy to understand.

@mborodii-prog

Copy link
Copy Markdown
Contributor Author

@ebhills now we support:

  1. Same number of defaults as columns → mapped 1:1
  2. One default (scalar or single-element list) → applied to all columns
  3. Mismatched lengths → ValueError

@thomasstvr thomasstvr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can you add a test (for both yaml and json) that uses more complex defaults and does not list outputs?

wrangles:
- convert.from_json:
    input:
        - header1
        - header2
    default:
        - my_output:
            key1: Val1
            key2: Val2
        - [this, is, a, list]

A few small changes, but looks good otherwise

Comment thread wrangles/recipe_wrangles/convert.py Outdated
Comment thread wrangles/recipe_wrangles/convert.py Outdated
Comment thread wrangles/recipe_wrangles/convert.py Outdated
Comment thread wrangles/recipe_wrangles/convert.py Outdated
Comment thread wrangles/recipe_wrangles/convert.py Outdated
@mborodii-prog

Copy link
Copy Markdown
Contributor Author

@thomasstvr Added two new tests (test_default_list_complex_values_no_output) to TestConvertFromJSON and TestConvertFromYAML covering complex per-column defaults (a nested dict and
a list) with no output specified, so the input columns are overwritten in place

Comment thread tests/recipes/wrangles/test_convert.py

@ebhills ebhills left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

the code looks correct, but the input data for a couple of the tests seem off. is this intentional to see if the default kicks in? the assertions do not seem to suggest that is the goal.

@ebhills ebhills left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Maybe an edge case bug? Maybe in the yaml parsing, not the defaults?

  - convert.from_yaml:
      input: 
        - Top *
      default: 
        - {}
        - []
        - ''


  - convert.to_yaml:
      input: 
        - Top *

I think this should be reversible, right? But row 2 comes out differently. I am running with batch size 1 so this may result of colm shifting i identified on the empty where df PR.
(first 3 columns are input)

raw input data:
Top 12 Top 23 Top 34
"Score: 0.295
Value: Blade Runner
" "[]
" "Score: 0.174
Value: Westworld
"
"Score: 0.234
Value: Interstellar
" "Score: 0.22
Value: Westworld
" "'
"
"Score: 0.291
Value: Interstellar
" "Score: 0.248
Value: Blade Runner
" "Score: 0.195
Value: Westworld
"

Now that I have pasted the data it makes me wonder why all of those "s are there.

The input columns were generated from running a previous version of the recipe where the first wrangle was from_json on this data:
Top 1 Top 2 Top 3
{"Score":0.295,"Value":"Blade Runner"} {"Score":0.174,"Value":"Westworld"}
{"Score":0.234,"Value":"Interstellar"} {"Score":0.22,"Value":"Westworld"}
{"Score":0.291,"Value":"Interstellar"} {"Score":0.248,"Value":"Blade Runner"} {"Score":0.195,"Value":"Westworld"}

@mborodii-prog mborodii-prog added this to the v1.20 milestone Jul 6, 2026
@mborodii-prog

Copy link
Copy Markdown
Contributor Author

@ebhills pls check now that behavior in QA

@mborodii-prog
mborodii-prog requested a review from ebhills July 13, 2026 11:39
@mborodii-prog

Copy link
Copy Markdown
Contributor Author

@ebhills @thomasstvr you can use recipe - convert.from_yaml:
input:
- Top *
default:
- {}
- []
- ''

  • convert.to_yaml:
    input:
    - Top *
    - for testing in QA.

@ebhills

ebhills commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

@thomasstvr - you also need to approve this one since you had requested changes

@ebhills
ebhills merged commit fdc5e02 into main Jul 26, 2026
34 of 45 checks passed
@ebhills
ebhills deleted the 985-enhancement-allow-list-of-onemultiple-defaults branch July 26, 2026 01:04
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.

[ENHANCEMENT] Allow list of one/multiple defaults

3 participants