Skip to content

Fix tree_map_with_path for namedtuples - #3674

Merged
zcbenz merged 4 commits into
ml-explore:mainfrom
chrismicah:codex/fix-tree-map-with-path-namedtuple
Jun 15, 2026
Merged

zcbenz merged 4 commits into
ml-explore:mainfrom
chrismicah:codex/fix-tree-map-with-path-namedtuple

Conversation

@chrismicah

@chrismicah chrismicah commented Jun 13, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Preserve namedtuple reconstruction in tree_map_with_path, matching tree_map behavior.
  • Add coverage for namedtuple tree_map_with_path paths, return type, and rest-argument mapping.
  • Add a pure-Python regression that can run without initializing MLX/Metal.

Tests

  • python -m unittest discover python/tests -p 'test_utils_pyonly.py' -v
  • python -m pytest python/tests/test_utils_pyonly.py -q
  • git diff --check upstream/main...HEAD

Not run locally

  • python -m pytest python/tests/test_tree.py -q blocked before collection in this headless session: RuntimeError: [metal::load_device] No Metal device available

@chrismicah

chrismicah commented Jun 13, 2026 •

Copy link
Copy Markdown
Contributor Author

Added one more regression in chrismicah/codex/fix-tree-map-with-path-namedtuple@243b1c94 covering tree_map_with_path over a NamedTuple with a matching rest tree, so the fix is not only single-tree reconstruction.

Verification:

  • Passed a Metal-free pure-Python import of python/mlx/utils.py exercising NamedTuple single-tree and rest-tree cases.
  • python python/tests/test_tree.py cannot run in this headless environment because installed MLX fails during import with [metal::load_device] No Metal device available before reaching test_tree.

@zcbenz

zcbenz commented Jun 13, 2026

Copy link
Copy Markdown
Member

Your text is not very readable.

@chrismicah
chrismicah force-pushed the codex/fix-tree-map-with-path-namedtuple branch from 243b1c9 to 1948966 Compare June 14, 2026 00:32
@chrismicah

Copy link
Copy Markdown
Contributor Author

Thanks, agreed. I cleaned up the PR description so it renders as normal Markdown sections instead of escaped newline text, and kept the test notes concise.

@zcbenz

zcbenz commented Jun 14, 2026 •

Copy link
Copy Markdown
Member
  • python -m pytest python/tests/test_tree.py -q blocked before collection in this headless session: RuntimeError: [metal::load_device] No Metal device available

You can disable metal backend when building.

@chrismicah

Copy link
Copy Markdown
Contributor Author

Thanks, that worked. I rebuilt this PR branch with Metal disabled and reran the requested test successfully.

Commands/results:

CMAKE_ARGS="-DMLX_BUILD_METAL=OFF" CMAKE_BUILD_PARALLEL_LEVEL=4 \
  /Users/chris.s/.pyenv/versions/3.11.7/bin/python -m pip install -e .
# Successfully installed mlx-0.32.0.dev20260614+19489666

/Users/chris.s/.pyenv/versions/3.11.7/bin/python -m pytest python/tests/test_tree.py -q
# 4 passed in 0.25s

/Users/chris.s/.pyenv/versions/3.11.7/bin/python -m pytest python/tests/test_utils_pyonly.py -q
# 1 passed in 0.03s

I also confirmed the rebuilt extension is the local PR build and Metal is disabled:

mlx /Users/chris.s/conductor/workspaces/ml-ide/caracas/.codex_tmp/mlx-pr3674/python/mlx/core.cpython-311-darwin.so
metal available False

@zcbenz

zcbenz commented Jun 15, 2026

Copy link
Copy Markdown
Member

test_utils_pyonly.py can be removed now?

@chrismicah

Copy link
Copy Markdown
Contributor Author

Removed it. The PR diff is now only:

  • python/mlx/utils.py
  • python/tests/test_tree.py

Re-ran after the cleanup:

/Users/chris.s/.pyenv/versions/3.11.7/bin/python -m pytest python/tests/test_tree.py -q
# 4 passed in 0.30s

@zcbenz zcbenz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

@zcbenz
zcbenz merged commit 28991d8 into ml-explore:main Jun 15, 2026
16 checks passed
@BrewTestBot BrewTestBot mentioned this pull request Jul 7, 2026
1 task done
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.

2 participants