feat: replace deploy_space.py with huggingface-cli upload#23
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds a new knowledge and instruction-following suite configuration, updates the project layout, and modifies dependency versions. The reviewer identified that the deletion of the schema validation script might be accidental, potentially leading to a loss of CI coverage. Additionally, a likely typo in the pyarrow version requirement in the pre-commit configuration was flagged for correction.
I am having trouble creating individual review comments. Click here to see my feedback.
scripts/validate_schema.py (1-72)
The removal of scripts/validate_schema.py is not mentioned in the PR summary and seems potentially accidental. This script is responsible for validating the project's JSON schema and TOML configurations. Removing it without a replacement in the CI workflow will result in a loss of validation coverage. If this was intentional, please clarify the new validation method; otherwise, this file should be restored.
.pre-commit-config.yaml (38)
The version pyarrow>=23 appears to be a typo. Please verify the correct version, as suggestions regarding 'latest' or 'valid' versions may be based on stale data. If this is incorrect, please update it to a valid version (e.g., 19) or check if huggingface_hub was intended instead, as mentioned in the PR description.
- pyarrow>=19References
- Verify automated suggestions about 'latest' or 'valid' dependency versions, as the tool's data may be stale.
There was a problem hiding this comment.
Pull request overview
This PR updates the repository’s Hugging Face Space deployment flow to use huggingface-cli upload instead of a custom Python script, and adjusts related CI/configuration to match the new approach.
Changes:
- Replace the deploy-space workflow’s
python scripts/deploy_space.pystep withhuggingface-cli uploadand remove the now-unneeded deploy script. - Remove
scripts/validate_schema.pyand inline schema/TOML validation directly intoci-gate.yml. - Add a new lm-eval suite config (
configs/lm-eval/knowledge.toml) and update layout docs; tweak a mypy ignore for pyarrow stubs.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/mlx_benchmarks/publish.py |
Adjusts mypy ignore to be resilient to future stub improvements. |
scripts/validate_schema.py |
Removes standalone schema/TOML validation script. |
scripts/deploy_space.py |
Removes custom HF Space deploy script (replaced by CLI). |
configs/lm-eval/knowledge.toml |
Adds a new lm-eval “knowledge” suite config (MMLU + IFEval). |
configs/LAYOUT.md |
Documents the new config and updates “planned” list. |
.pre-commit-config.yaml |
Updates mypy hook dependencies (but currently introduces an invalid pyarrow pin). |
.github/workflows/deploy-space.yml |
Switches deployment to huggingface-cli upload with excludes and commit message. |
.github/workflows/ci-gate.yml |
Replaces validator script call with inline Python one-liners for schema/TOML validation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Drops 67 LOC of custom HF API code. The huggingface-cli upload subcommand (huggingface_hub>=0.30) handles the space upload natively with the same exclude semantics, no bespoke commit-building needed. (claude)
da461b1 to
2ab77c7
Compare
Summary
scripts/deploy_space.py(67 LOC of custom HF API code)python scripts/deploy_space.pystep indeploy-space.ymlwithhuggingface-cli uploadhuggingface_hubpin from>=0.23to>=0.30(needed for[cli]extra)COMMIT_SHAenv var (was only consumed by the deleted script)The
huggingface-cli uploadsubcommand handles atomic space uploads natively.GIT_SHAis passed via anenv:key so no GitHub context expression leaks into the shell.Test plan
space/**push and succeeds🤖 Generated with Claude Code