|
| 1 | +# Contributing to `mkl_fft` |
| 2 | + |
| 3 | +This document covers the development workflow: how to get a working build, how |
| 4 | +to run the checks, and what to include in a pull request. |
| 5 | + |
| 6 | +For end-user installation and API usage, see [README.md](README.md). For a map |
| 7 | +of the source tree, see [`AGENTS.md`](AGENTS.md), which links to the local |
| 8 | +`AGENTS.md` files in directories that have their own rules. Security |
| 9 | +vulnerabilities go through the process in [SECURITY.md](SECURITY.md). |
| 10 | + |
| 11 | +--- |
| 12 | + |
| 13 | +## Development setup |
| 14 | + |
| 15 | +Building requires a C compiler, oneMKL headers and libraries (`mkl-devel`), and |
| 16 | +NumPy. A conda environment is the least surprising way to get them: |
| 17 | + |
| 18 | +```sh |
| 19 | +# add python=X.Y to target a specific interpreter |
| 20 | +conda create -n mkl_fft-dev -c conda-forge python pip mkl-devel numpy \ |
| 21 | + meson-python ninja cmake cython pytest |
| 22 | +conda activate mkl_fft-dev |
| 23 | +``` |
| 24 | + |
| 25 | +Then build in place, which reuses the environment's MKL and NumPy: |
| 26 | + |
| 27 | +```sh |
| 28 | +pip install -e . --no-build-isolation --verbose |
| 29 | +``` |
| 30 | + |
| 31 | +`pyproject.toml` defines the supported Python range, and |
| 32 | +`.github/workflows/build_pip.yml` is canonical for the versions CI covers. |
| 33 | + |
| 34 | +SciPy and `mkl-service` are optional. They are needed only for the |
| 35 | +`mkl_fft.interfaces.scipy_fft` adapter, and the tests that exercise it are |
| 36 | +skipped without them. To use or test the SciPy interface, add both to the |
| 37 | +environment: |
| 38 | + |
| 39 | +```sh |
| 40 | +conda install -c conda-forge scipy mkl-service |
| 41 | +``` |
| 42 | + |
| 43 | +The matching pip extras are declared in `pyproject.toml`: `scipy_interface` for |
| 44 | +the SciPy adapter, `test` for `pytest` plus both packages, and `benchmark` for |
| 45 | +the ASV suite. |
| 46 | + |
| 47 | +`README.md` documents the non-editable install paths, including the isolated |
| 48 | +build that resolves its own `mkl` and `numpy`. |
| 49 | + |
| 50 | +### Rebuilding |
| 51 | + |
| 52 | +`meson-python` rebuilds the extension on import for editable installs, so |
| 53 | +editing `.pyx`, `.c.src`, or `meson.build` and rerunning `pytest` is usually |
| 54 | +enough. Generated sources and the compiled extension live under `build/<tag>/` |
| 55 | +rather than in the source tree. If a build gets into a bad state, `rm -rf build` |
| 56 | +and reinstall. |
| 57 | + |
| 58 | +## Running the checks |
| 59 | + |
| 60 | +```sh |
| 61 | +pytest mkl_fft/tests # test suite |
| 62 | +pre-commit run --all-files # lint and format hooks |
| 63 | +``` |
| 64 | + |
| 65 | +Install the hooks once with `pre-commit install` and they run on each commit. |
| 66 | +`.pre-commit-config.yaml` is the source of truth for the tooling. |
| 67 | + |
| 68 | +Opening a pull request also runs CI, which builds and tests the package across |
| 69 | +platforms and Python versions and runs various lint and static-analysis checks. |
| 70 | + |
| 71 | +## Code style |
| 72 | + |
| 73 | +Style is loose, and the pre-commit hooks enforce most of it: |
| 74 | + |
| 75 | +- Python is formatted with `black` and `isort`, with a line length of 80. |
| 76 | +- Cython is not touched by `black`. `isort` sorts its imports, `cython-lint` |
| 77 | + checks it against the same 80-column limit, and string literals use double |
| 78 | + quotes. |
| 79 | +- C sources follow the repository's `.clang-format`. |
| 80 | +- Otherwise, match the surrounding code. |
| 81 | + |
| 82 | +## Dos and don'ts |
| 83 | + |
| 84 | +**Do** |
| 85 | + |
| 86 | +- Keep changes atomic and single-purpose. |
| 87 | +- Preserve NumPy/SciPy FFT compatibility. This package is used as a drop-in |
| 88 | + replacement, so a behavioral difference is a bug even when the new behavior is |
| 89 | + arguably better. Call out an intentional break in the PR. |
| 90 | +- Add tests in `mkl_fft/tests/` alongside behavior changes, and a regression |
| 91 | + test with every bug fix. |
| 92 | +- Keep tests deterministic. |
| 93 | +- Edit the `*.c.src` templates in `mkl_fft/src/` for C backend changes. The |
| 94 | + `.c` files are generated from them on every build. |
| 95 | +- Keep patching reversible and observable: anything installed can be |
| 96 | + uninstalled, and `is_patched()` reports the truth. |
| 97 | +- Cite the source-of-truth file for mutable details: `pyproject.toml`, |
| 98 | + `meson.build`, `conda-recipe*/meta.yaml`, `.github/workflows/`. |
| 99 | +- Give benchmark numbers reproducible context — hardware, versions, and the |
| 100 | + command you ran. |
| 101 | + |
| 102 | +**Don't** |
| 103 | + |
| 104 | +- Commit generated artifacts, or hand-edit a generated `.c`. |
| 105 | +- Hardcode versions, build flags, CI matrices, or channel URLs in documentation. |
| 106 | +- Assert on timing or throughput in the test suite. |
| 107 | +- Refactor `_vendored/` opportunistically. Keep local diffs minimal and send |
| 108 | + fixes upstream where you can. |
| 109 | +- Introduce ISA-specific assumptions outside explicit build configuration. |
| 110 | + |
| 111 | +## Submitting a change |
| 112 | + |
| 113 | +Work on a branch: the `no-commit-to-branch` hook blocks direct commits to |
| 114 | +`master` and `maintenance/*`. |
| 115 | + |
| 116 | +If the change is user-visible — behavior, API, packaging, or build output — add |
| 117 | +a `CHANGELOG.md` entry under `## [dev]` in the matching section, with a |
| 118 | +`[gh-NNN](https://github.com/IntelPython/mkl_fft/pull/NNN)` link. The format |
| 119 | +follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) and the project |
| 120 | +follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). Docs, |
| 121 | +tooling, and CI-only changes are usually left out. |
| 122 | + |
| 123 | +Then open the PR and fill in the template, including what you verified locally |
| 124 | +and what you left to CI. |
| 125 | + |
| 126 | +By contributing you agree that your contributions are licensed under the |
| 127 | +BSD-3-Clause terms in [LICENSE.txt](LICENSE.txt). |
0 commit comments