task: agent-ready prep - #379
jharlow-intel wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The documented lint setup is not executable as written, and two documentation nits remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Documentation-only preparation for contributors, with no runtime, build, or packaging changes.
Changes:
- Adds contributor setup, testing, and workflow guidance.
- Adds a pull-request template.
- Links contributor guidance from
README.md. - Ignores local Claude Code settings.
File summaries
| File | Summary |
|---|---|
README.md |
Links to contributor guidance. |
CONTRIBUTING.md |
Documents development workflows and project structure. |
.gitignore |
Ignores local Claude settings. |
.github/pull_request_template.md |
Adds PR submission prompts and checklist. |
Review details
Suppressed comments (1)
CONTRIBUTING.md:19
- The documented lint command cannot run in the environment created here:
pre-commitis not installed by this command and is not part of the[test]extra. Add it to the environment (or document a separate install step) so the setup and checks are executable end to end.
meson-python ninja cmake cython pytest scipy mkl-service
- Files reviewed: 3/4 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
ndgrigorian
left a comment
ndgrigorian
left a comment
There was a problem hiding this comment.
Frankly I have no issue with anything in this PR besides those nits in the contributing document, LGTM
antonwolfy
left a comment
antonwolfy
left a comment
There was a problem hiding this comment.
Few minor nits, but in overall LGTM
|
|
||
| Style is loose, and the pre-commit hooks enforce most of it: | ||
|
|
||
| - Python and Cython are formatted with `black` and `isort`, with a line length |
There was a problem hiding this comment.
black never touches Cython files, only isort (cython) and cython-lint handle Cython
| - Treat `asv.conf.json` as canonical for ASV settings; treat `README.md` as | ||
| canonical for what each module covers. | ||
| - Comparability across machines depends on the thread default in | ||
| `benchmarks/__init__.py` and the DFTI warmup in each `setup`. Changing either |
There was a problem hiding this comment.
The peak-memory benchmarks (PeakMemFFT1D, etc. in bench_memory.py) define no setup and inherit the base setup, which only builds the input array — no warmup call.
| description: | | ||
| Output of: | ||
| ``` | ||
| python -c "import mkl_fft, numpy, mkl; print(mkl_fft.__version__); print(numpy.__version__); print(mkl.get_version_string())" |
There was a problem hiding this comment.
mkl.get_version_string() requires mkl-service, which is optional dep for mkl_fft and might not be installed be default
Description
Adds
CONTRIBUTING.mdand a pull-request template, gitignores developer-localClaude Code settings, and links CONTRIBUTING from the README.
Prompted by an AgentReady readiness scan, which flagged the missing developer
docs and PR template as its only findings (96/100 → 100/100).
Verification
pytest mkl_fft/tests— 971 passed, 2 skipped (Python 3.12,NumPy 2.5.3, Linux). The setup steps in th
end to end:
conda create,pip install -e ".[test]" --no-build-isolation,and the
rm -rf build+ reinstall recovery path.pre-commit run --files CONTRIBUTING.md README.md .gitignore .github/pull_request_template.md— all apChecklist
mkl_fft/tests/; bug fixes have a regression test.