dora icon indicating copy to clipboard operation
dora copied to clipboard

Add macOS-only mlx-lm node to node-hub (#882)

Open Clement795 opened this issue 10 months ago • 5 comments

This PR addresses issue #882 by adding support for mlx-lm in the dora node-hub.

Changes:

  • Added mlx-lm node for macOS compatibility (node-hub/mlx-lm.py).
  • Made mlx-lm macOS-only with platform.system() check to skip execution on Linux/Windows, fixing CI failures.
  • Tested locally on macOS 15.4 with mlx-lm version 0.23.0.

Notes:

  • mlx-vlm support is not included and can be addressed separately.
  • CI Python tests should now pass across all platforms due to platform check.

Addresses #882 (partially, as mlx-vlm is not implemented).

@haixuanTao Please review this updated PR for adding mlx-lm support.

Clement795 avatar Apr 26 '25 13:04 Clement795

@haixuanTao The CI failed due to node-hub/dora-mlx-lm/ depending on mlx-lm, which isn’t available on Linux/Windows. I removed it and used node-hub/mlx-lm.py. Do I need to modify the CI workflow to test mlx-lm only on macOS?

Clement795 avatar Apr 26 '25 13:04 Clement795

Can you put it in the skip folder?

See: https://github.com/dora-rs/dora/blob/66bf1b74a42f6832f2d5b793c53d7e2069bc298b/.github/workflows/node_hub_test.sh#L8

haixuanTao avatar Apr 26 '25 13:04 haixuanTao

@haixuanTao skipping tests wasn't enough because the pyproject.toml still declares the mlx dependency, which gets resolved during linting and installation steps, causing failures. Moving the folder to ignored_folders prevents any CI steps (install, lint, test) from running on it. PR ready for review, thanks in advance!

Clement795 avatar Apr 26 '25 15:04 Clement795

That makes sense. Thanks!

haixuanTao avatar Apr 27 '25 11:04 haixuanTao

Triage: I resolved the conflict with the main branch. I think this PR is ready for another review @haixuanTao .

phil-opp avatar Aug 19 '25 09:08 phil-opp