fix(packaging): guard torch extras on intel macos - #2011
Conversation
PR governanceThis PR follows the template and is marked ready for human review. |
JerrettDavis
left a comment
There was a problem hiding this comment.
Thanks for the targeted fix and tests. The local checks pass, but this does not fully solve #1931 for headroom-ai[all] on Intel macOS.
The direct torch entries in [ml] and [voice] are guarded, but [all] still expands to [memory] and [evals], and both include sentence-transformers. In the lock metadata, sentence-transformers has an unguarded dependency on torch (uv.lock has name = "sentence-transformers" with { name = "torch" } under that package), so an Intel macOS [all] resolve can still reach torch transitively.
I verified this from the PR branch with a resolver-style check against pyproject.toml: for sys_platform=darwin and platform_machine=x86_64, [all] still selects sentence-transformers=True and datasets=True, while only direct torch=False. That leaves the original failure path open.
Passing local checks:
python -m pytest tests/test_optional_dependencies.py -qpython -m ruff check tests/test_optional_dependencies.pypython -m ruff format --check tests/test_optional_dependencies.py pyproject.tomlgit diff --check headroomlabs/main...HEAD
Please update the fix to guard the torch-bearing extras from [all] on Intel macOS, or otherwise prove the full transitive [all] resolve no longer selects torch there.
bda6fdb to
381b64f
Compare
|
Addressed the remaining Intel macOS [all] path. [memory] and [evals] now guard sentence-transformers with the same darwin/x86_64 exclusion, and uv.lock metadata reflects that [all] no longer selects the torch-bearing sentence-transformers dependency on Intel macOS.\n\nChecks:\n- RED: python3 -m pytest tests/test_optional_dependencies.py -q before the fix -> [all] selected sentence-transformers from the locked torch-bearing set\n- GREEN: python3 -m pytest tests/test_optional_dependencies.py -q -> 1 passed\n- uv run ruff check tests/test_optional_dependencies.py\n- uv run ruff format --check tests/test_optional_dependencies.py pyproject.toml\n- git diff --check upstream/main...HEAD |
JerrettDavis
left a comment
There was a problem hiding this comment.
Looks good now. The Intel macOS [all] path is covered: direct torch extras are guarded, and the torch-bearing sentence-transformers entries from [memory] and [evals] are also excluded for darwin/x86_64.
Verified locally:
uv run pytest tests/test_optional_dependencies.py -q-> 1 passeduv run ruff check tests/test_optional_dependencies.pyuv run ruff format --check tests/test_optional_dependencies.py pyproject.tomlgit diff --check FETCH_HEAD...HEAD
CI is green as well.
`headroom wrap omp` lazily imports yaml to parse and merge omp's models.yml registry, but pyyaml was never declared, so a bare `pip install headroom-ai` (no extras) fails at wrap time with ModuleNotFoundError. Dev and test environments masked this: pyyaml arrives transitively through dev/ml extras (pre-commit, transformers). CI surfaced it as the lint failure on the merge commit: the lint job installs no project dependencies, so mypy flags the yaml import as import-untyped (typeshed publishes stubs for it). - pyproject.toml: add pyyaml>=6.0 to core dependencies - uv.lock: surgically add pyyaml to the headroom-ai dependencies and requires-dist metadata (same approach as headroomlabs-ai#2011; a full `uv lock` regeneration would undo that PR's intel-mac torch guard) - omp/runtime.py: annotate the import with type: ignore[import-untyped] following the psutil precedent (the lint env installs no deps)
## Description Closes headroomlabs-ai#1931 Guard the `ml` and `voice` `torch` optional dependencies on macOS x86_64 so `headroom-ai[all]` remains resolvable on Intel Macs where PyTorch does not publish compatible wheels for this version floor. The lockfile metadata is updated with the same markers. ## Type of Change - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change that adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] Documentation update - [ ] Refactoring - [ ] Performance improvement - [ ] Test update - [ ] Other ## Changes Made - Added macOS x86_64 environment markers to `torch` in the `ml` and `voice` extras. - Updated `uv.lock` optional dependency metadata to match the guarded extras. - Added a packaging regression test that checks `[all]` keeps `ml` and `voice` while guarding `torch` on macOS x86_64. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check`) - [x] Formatting verified (`ruff format --check`) - [ ] Manual testing performed ### Test Output ```text $ python3 -m pytest tests/test_optional_dependencies.py -q collected 1 item tests/test_optional_dependencies.py . [100%] ============================== 1 passed in 0.26s =============================== $ .venv/bin/ruff check tests/test_optional_dependencies.py All checks passed! $ .venv/bin/ruff format --check tests/test_optional_dependencies.py pyproject.toml 1 file already formatted ``` ## Test verification (RED -> GREEN) RED, with the `torch` markers temporarily removed from `pyproject.toml`: ```text tests/test_optional_dependencies.py F [100%] FAILED tests/test_optional_dependencies.py::test_all_extra_does_not_require_torch_on_macos_x86_64 E assert False ``` GREEN, with this patch applied: ```text tests/test_optional_dependencies.py . [100%] ============================== 1 passed in 0.26s =============================== ``` ## Real Behavior Proof - Environment: Linux, Python 3.12.3, pytest 9.1.1, ruff 0.14.14. - Exact command / steps: Removed the environment markers from `torch`, ran the new packaging test, restored the markers, and reran the test plus targeted ruff checks. - Observed result: The test fails without the macOS x86_64 guard and passes once the `ml` and `voice` `torch` requirements are guarded. - Not tested: Full `uv run pytest`, full-project `uv run ruff check .`, full-project `uv run ruff format --check .`, and `uv run mypy headroom` were not run locally for this targeted packaging change. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Checklist - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my code - [x] I have added tests that prove my fix is effective - [x] New and existing targeted tests pass locally with my changes - [x] Any dependent changes have been merged and published in downstream modules ## Screenshots (if applicable) N/A ## Additional Notes No new dependency is added; this only narrows when the existing `torch` optional dependency is selected.
Description
Closes #1931
Guard the
mlandvoicetorchoptional dependencies on macOS x86_64 soheadroom-ai[all]remains resolvable on Intel Macs where PyTorch does not publish compatible wheels for this version floor. The lockfile metadata is updated with the same markers.Type of Change
Changes Made
torchin themlandvoiceextras.uv.lockoptional dependency metadata to match the guarded extras.[all]keepsmlandvoicewhile guardingtorchon macOS x86_64.Testing
pytest)ruff check)ruff format --check)Test Output
Test verification (RED -> GREEN)
RED, with the
torchmarkers temporarily removed frompyproject.toml:GREEN, with this patch applied:
Real Behavior Proof
torch, ran the new packaging test, restored the markers, and reran the test plus targeted ruff checks.mlandvoicetorchrequirements are guarded.uv run pytest, full-projectuv run ruff check ., full-projectuv run ruff format --check ., anduv run mypy headroomwere not run locally for this targeted packaging change.Review Readiness
Checklist
Screenshots (if applicable)
N/A
Additional Notes
No new dependency is added; this only narrows when the existing
torchoptional dependency is selected.