fix(util): keep None as null in to_pod instead of the string "None"

[kevin9327]

PR Type

• Bug fix
• New feature
• Core Runtime change (higher bar -- see CONTRIBUTING.md)
• Docs / tooling
• Refactoring

Summary

to_pod() turned None into the string "None". Because _graph_info
serializes every decorator's attribute dict through to_pod(), and most
decorators default their optional attributes to None, a plain
@resources(cpu=2, memory=8192) was recorded as "gpu": "None" instead of
"gpu": null. "None" is truthy, so anything reading those attributes back
concludes a GPU was requested.

Issue

No existing issue -- I hit this while reading a run's _graph_info and can file
one first if you'd prefer that order.

Reproduction

Runtime: local

Commands to run:

pip install -e .
python repro_to_pod.py

repro_to_pod.py -- goes through the same output_steps() call that
FlowSpec._graph_info uses (metaflow/flowspec.py:535, metaflow/graph.py:618):

import json
from metaflow import FlowSpec, step, resources


class ResourcesFlow(FlowSpec):
    @resources(cpu=2, memory=8192)
    @step
    def start(self):
        self.next(self.end)

    @step
    def end(self):
        pass


steps_info, _ = ResourcesFlow._graph.output_steps()
res = [d for d in steps_info["start"]["decorators"] if d["name"] == "resources"][0]
print(json.dumps(res["attributes"], indent=2))
gpu = res["attributes"]["gpu"]
print("gpu:", repr(gpu), "| bool(gpu):", bool(gpu))

Where evidence shows up: the _graph_info artifact -- so also
current.graph, the graph_info passed to cards (card_cli.py:721), and
Run(...)["_graph_info"].data via the client.

Before (on master)

{
  "cpu": 2,
  "gpu": "None",
  "disk": "None",
  "memory": 8192,
  "shared_memory": "None"
}
gpu: 'None' | bool(gpu): True

After

{
  "cpu": 2,
  "gpu": null,
  "disk": null,
  "memory": 8192,
  "shared_memory": null
}
gpu: None | bool(gpu): False

Root Cause

metaflow/util.py:563. to_pod() passes the JSON-native scalars through
unchanged and ends with a catch-all return str(value):

if isinstance(value, (str, int, float)):   # bool included, it subclasses int
    return value
...
return str(value)

None matches none of the branches above, so it reaches str(value) and
becomes "None". The invariant the function is supposed to hold -- "values
that are already plain-old-data come back unchanged" -- is broken for the one
POD value that isn't a str, a number or a container.

The reason it shows up so widely is that node_to_dict() in graph.py:618
does "attributes": to_pod(deco.attributes), and deco.attributes is the
decorator's defaults dict updated with whatever the user passed. Unset
optional attributes stay None. For @resources that is
gpu/disk/shared_memory; the same holds for other decorators that default
optional attributes to None.

Why This Fix Is Correct

Two added lines, ahead of the existing scalar branch:

if value is None:
    return value

None is valid POD and serializes to JSON null, so returning it unchanged is
exactly what the surrounding branches already do for str, int, float and
bool. Nothing else in the function changes, and no other value can reach the
new branch.

Failure Modes Considered

1. Something downstream relies on the string. I grepped for consumers
   comparing against "None"; the only == "None" in metaflow/ is
   cmd/develop/stub_generator.py:499, which inspects type annotations and
   never sees to_pod output. Card modules render _graph_info attributes for
   display rather than matching on them.
2. Backward compatibility of stored artifacts. This only changes what is
   written into new _graph_info artifacts. Runs recorded before this change
   keep their "None" strings and still load, since nothing parses the value --
   a reader that used to get a truthy "None" now gets a falsy None, which is
   the intended reading.
3. bool regressing to the catch-all. bool subclasses int, so
   True/False still hit the existing isinstance(value, (str, int, float))
   branch and are unaffected; the new branch is is None, not falsiness, so
   0, "" and False are untouched. Covered by the existing
   test_to_pod_primitives.
4. Dict keys. to_pod recurses into keys as well
   ({to_pod(k): to_pod(v) ...}). A None key now stays None instead of
   becoming "None"; that is still a valid Python dict key, and json.dumps
   coerces it to "null" the same way it coerces other non-string keys.

Tests

• Unit tests added/updated
• Reproduction script provided
• CI passes
• If tests are impractical: explain why below and provide manual evidence above

Added to test/unit/test_to_pod.py:

• test_to_pod_none_is_preserved
• test_to_pod_none_nested_in_dict_and_list
• test_to_pod_none_is_falsy_after_conversion

and to test/unit/test_graph_structure.py (next to the existing
output_steps() / to_pod regression test):

• test_unset_decorator_attributes_stay_null_in_output_steps

# before, on unmodified master, metaflow/ untouched
$ python -m pytest test_to_pod.py test_graph_structure.py -q
FAILED test_to_pod.py::test_to_pod_none_is_preserved - AssertionError: assert 'None' is None
FAILED test_to_pod.py::test_to_pod_none_nested_in_dict_and_list
FAILED test_to_pod.py::test_to_pod_none_is_falsy_after_conversion
FAILED test_graph_structure.py::test_unset_decorator_attributes_stay_null_in_output_steps
4 failed, 62 passed

# after
$ python -m pytest test_to_pod.py test_graph_structure.py -q
66 passed

Pinned against regressions -- the whole of test/unit before and after the
change, same invocation:

before: 21 failed, 499 passed, 20 skipped, 103 errors
after:  21 failed, 503 passed, 20 skipped, 103 errors

The delta is exactly the four new tests. In the interest of being straight
about it: those 21 failures and 103 errors are pre-existing on my machine and
are not related to this change. I develop on Windows, which Metaflow does not
support (setup.py classifiers list only macOS and Linux) -- several core
modules import fcntl at module scope, so every test that actually spawns a run
fails locally for me regardless of this patch. The counts are identical before
and after, and the pure-logic tests that matter here run fine. I am relying on
CI for the Linux/macOS matrix.

black 25.12.0 with the target list from .pre-commit-config.yaml reports all
three files unchanged.

Non-Goals

I did not touch the catch-all return str(value) for genuinely non-POD values
(that is what makes callables and arbitrary objects safe to embed), and I did
not change any decorator's defaults.

AI Tool Usage

• No AI tools were used in this contribution
• AI tools were used (describe below)

Claude Code, used to grep the callers of to_pod and to draft this description.
The reproduction, the before/after test runs and the counts above are from
commands I actually ran; I understand why None fell through to str(value)
and why moving the check ahead of the scalar branch is safe.

[greptile-apps]

Greptile Summary

This PR corrects to_pod() so Python None remains a JSON-compatible null rather than becoming the truthy string "None".

• Preserves null values recursively in decorator attribute structures.
• Adds focused primitive and nested-container regression coverage.
• Verifies unset resource attributes remain null in generated graph metadata.

Confidence Score: 5/5

The PR appears safe to merge, with the narrowly scoped null-preservation fix covered at both utility and graph-output levels.

The new branch preserves a valid POD value without affecting other scalar or fallback conversions, and no repository consumer was found to depend on the previous "None" representation.

Important Files Changed

Filename	Overview
metaflow/util.py	Adds an explicit None passthrough before existing scalar and container conversion logic.
test/unit/test_to_pod.py	Covers direct, nested, and falsiness behavior for preserved None values.
test/unit/test_graph_structure.py	Verifies unset resource decorator attributes remain null in output_steps() metadata.

_{Reviews (1): Last reviewed commit: "fix(util): keep None as null in to_pod i..." | Re-trigger Greptile}

[Shriprasad-P]

test permission

[Shriprasad-P]

Independently reproduced on current master (72591a0): to_pod(None) is "None", and @resources(cpu=2, memory=8192) serializes gpu/disk/shared_memory as the truthy string "None" through output_steps().

The is None passthrough in to_pod is the right place and the right test (is None, not falsiness). Unset resource attributes become JSON null on this head; False/0/""/callables are unchanged. Production to_pod callers are graph.py and flowspec.py graph metadata; no in-repo consumer matches to_pod output against "None".

Focused tests: 66 passed (test/unit/test_to_pod.py, test/unit/test_graph_structure.py). Black/pre-commit on the three changed files passed locally.

GitHub Actions Test (pre-commit + Python matrix) has not run on this SHA; only Greptile Review is present. That is not a code defect in this diff.

No substantive review findings. Approving the change.

[Shriprasad-P]

Reviewed the to_pod change and the related regression coverage. I reproduced the current behavior where None is serialized as the string "None" and verified that this patch correctly preserves it as None, producing JSON null while leaving other scalar behavior unchanged.

I also checked the graph/decorator path and relevant compatibility cases, and the focused tests and pre-commit checks pass locally. I did not find any blocking issues with this change.

[Shriprasad-P]

Review

Clear bugfix: to_pod(None) must stay JSON-null instead of becoming the string "None", especially for unset @resources attributes in _graph_info.

Strengths

• Early return is small and correct; nested cases work because recursion hits the same path.
• Unit tests cover top-level, nested dict/list, and the truthiness pitfall that motivated the change.

Approve.

[Shriprasad-P]

Review

Clear bugfix: to_pod(None) must stay JSON-null instead of becoming the string "None", especially for unset @resources attributes in _graph_info.

Strengths

• Early return is small and correct; nested cases work because recursion hits the same path.
• Unit tests cover top-level, nested dict/list, and the truthiness pitfall that motivated the change.

Approve.