Uh oh!
There was an error while loading. Please reload this page.
python-test: add a needs-dlpack input to skip the dlpack source build - #10
Merged
Conversation
The reusable test workflow unconditionally installs fastfields-dlpack from source (and the clang toolchain that build needs) for every calling repo except fastfields-dlpack itself. That is right for the wrappers, which all call the compiled bindings, but wrong for a package that does not depend on dlpack at all: it would build the entire C++ chain -- recursive submodule checkout plus a clang compile -- on every runner in the matrix, macOS included, to test code that imports nothing but the stdlib. Add a `needs-dlpack` boolean input defaulting to `true`, so every existing caller (numpy / torch / cupy / dlpack, none of which pass a `with:` block) is unaffected, and gate on it both the cross-repo install and the three per-OS toolchain steps, which exist only to serve that build. The first caller to pass `false` is the incoming fastfields-helpers, the pure-Python normalisation/enum package being split out of fastfields-dlpack. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z
balbasty pushed a commit
to fastfields/fastfields-helpers
that referenced
this pull request
Aug 18, 2026
Extracted from fastfields-dlpack's fastfields/dlpack/_helpers.py (verified byte-identical function bodies against the source it was seeded from) so Python-only fixes to these enums/normalisers ship without a compiled-wheel release cycle across every platform. Zero runtime dependencies -- stdlib only, no numpy/torch/cupy, no fastfields.dlpack. Mirrors fastfields-numpy's skeleton (not fastfields-dlpack's): plain versioningit, no compiled extension, release.yaml calls the reusable python-release.yml with compiled: false. test.yaml passes needs-dlpack: false (fastfields/.github#10) so CI does not build the unrelated C++ chain just to test stdlib-only code. 9 public symbols: Spline, Bound, as_spline, as_bound, normalize_shape, infer_ndim, check_ndim, resolve_out_spatial, anchor_scale_shift. tests/test_helpers.py ports test_bindings.py::test_shared_helpers from fastfields-dlpack, split into per-topic functions, plus a stdlib-only import guard. Verified before push: pip install ".[test]" (Requires: empty), pytest run out-of-tree matching the CI invocation (6 passed), ruff check + ruff format --check + codespell clean at the CI-pinned versions (ruff 0.15.8, codespell 2.4.3), and no fastfields/__init__.py installed (PEP 420 intact). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z
balbasty added a commit
that referenced
this pull request
Aug 18, 2026
The layer diagram in CONTRIBUTING.md enumerated the six original repos but predates fastfields-helpers -- the pure-Python enums/normalisation package extracted from fastfields-dlpack. Add it as an independent sibling (neither depends on the other) consumed by the numpy/torch/cupy wrappers and the umbrella fastfields package. Also note in the Workflows section that fastfields-helpers is the one repo that needs neither fastfields-dlpack nor the C++ build toolchain, via the reusable workflow's new `needs-dlpack: false` input (#10). Factual housekeeping only, not a design change -- the cu124 wheel-lane naming drift a few sections down is pre-existing and out of scope here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z
balbasty added a commit
that referenced
this pull request
Aug 18, 2026
* docs: add fastfields-helpers to the layer diagram and CI note The layer diagram in CONTRIBUTING.md enumerated the six original repos but predates fastfields-helpers -- the pure-Python enums/normalisation package extracted from fastfields-dlpack. Add it as an independent sibling (neither depends on the other) consumed by the numpy/torch/cupy wrappers and the umbrella fastfields package. Also note in the Workflows section that fastfields-helpers is the one repo that needs neither fastfields-dlpack nor the C++ build toolchain, via the reusable workflow's new `needs-dlpack: false` input (#10). Factual housekeeping only, not a design change -- the cu124 wheel-lane naming drift a few sections down is pre-existing and out of scope here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z * Fix: restore full CONTRIBUTING.md content (previous commit accidentally wrote a placeholder) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
python-test.ymlunconditionally installsfastfields-dlpackfrom source for every calling repo exceptfastfields-dlpackitself, along with the per-OS clang toolchain that build requires.That is correct for the wrappers —
fastfields-numpy/-torch/-cupyall call the compiled bindings, so they genuinely need dlpack present. It is wrong for a package that does not depend on dlpack at all: such a repo would do a recursive submodule checkout and a full clang compile of the entire C++ chain on every runner in the matrix (6 jobs, macOS included) purely to test code that imports nothing but the stdlib.This is the blocking prerequisite for
fastfields-helpers, the pure-Python normalisation/enum package being split out offastfields-dlpackso Python-only fixes no longer require a compiled-wheel release across every platform.fastfields-helperswill havedependencies = [].What
needs-dlpackbooleanworkflow_callinput, defaulting totrue.Install fastfields-dlpack from sourcestep, and the three per-OSInstall build toolchainsteps, which exist only to serve that build.Gating the toolchain steps as well as the install is deliberate and safe: the workflow's own comment already states they are there for "the C++ build that fastfields-dlpack needs".
fastfields-dlpackitself keeps the defaulttrue, so it still gets clang for its ownpip install ".[test]"extension build.Compatibility
No existing caller changes, and none needs to. Verified against every repo that uses this workflow:
fastfields-numpyuses:+secrets: inherit, nowith:true— unchangedfastfields-torchtrue— unchangedfastfields-cupytrue— unchangedfastfields-dlpacktrue— unchanged (still gets clang for its own build)fastfields(meta)test.yamlThe only caller that will pass
needs-dlpack: falseis the incomingfastfields-helpers.Workflow YAML parses clean; the input resolves as
type: boolean, default: trueand all four gated steps carry the expectedif:expressions.🤖 Generated with Claude Code
https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z
Generated by Claude Code