Add nanobind stubgen (fixes mypy failures from #4220) - #4342
Merged
Conversation
* add cpp stub files * fix nanobind OUTPUT path Note that these OUTPUT are not actually passed to the stub generator and purely used for dependency management within CMake.
…inx into jhale/nanobind-autogen-stubs
The auto-generated dolfinx.cpp stubs made mypy see real, precise types for the compiled extension for the first time, surfacing ~446 errors in the "Build and test" CI job (the only mypy invocation that actually installs dolfinx before running mypy, so the only one exercising the generated stubs). Root causes and fixes: - Unix stub generation imported the compiled module as bare `cpp` instead of `dolfinx.cpp`, so nanobind wrote cross-submodule references like `import cpp.la`, which doesn't exist and silently resolved to Any under mypy, masking real errors and producing bogus "overload can never match" diagnostics. Stage the built module under a throwaway dolfinx/ package dir before invoking nanobind_add_stub so the module's real __name__ is dolfinx.cpp. - `_IntegralType`/`MPICommWrapper` bindings used names or const_name() values with no resolvable Python type, breaking stub cross-refs. - `FiniteElement`/`AdjacencyList` `__eq__` bindings exposed the raw C++ operator==, violating object.__eq__'s Liskov contract; wrapped in a lambda with an isinstance guard instead. - Dead, unreachable overloads (complex instantiations of interpolation_matrix/discrete_curl/discrete_gradient that are always shadowed by the real ones, and a redundant read_geometry_data registration) removed. - Missing nanobind/stl/map.h and .../string.h includes were causing stubgen to fall back to invalid raw C++ type strings. - ~250 Python-side errors are the same runtime-safe-but-statically- unverifiable scalar-type dispatch pattern already handled elsewhere in this PR (fem/petsc.py); fixed with matching `# type: ignore`. A handful were real bugs instead: wrong return-type annotations in forms.py, and dispatch-table locals missing an explicit Union annotation. - The ~24 remaining errors are genuine ecosystem-level gaps confirmed via isolated repros (mypy can't disambiguate numpy dtype/rank in NDArray annotations; nanobind has no std::reference_wrapper caster; basix's C++ types aren't stub-resolvable across the package boundary) rather than dolfinx bugs. Suppressed per-file via a new nanobind stub pattern file (stub_patterns.txt) that injects a scoped `# mypy: disable-error-code=...` into just the affected generated stubs. Verified: mypy clean (matching the failing job's exact PETSc/ADIOS2 config), ruff/clang-format clean, and a second full build with PETSc+SLEPc+ADIOS2+ParMETIS+SuperLU_DIST (-Werror) compiles cleanly with runtime sanity checks passing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…n-stubs # Conflicts: # .github/workflows/ci-spack.yml # python/dolfinx/fem/__init__.py # python/dolfinx/fem/assemble.py # python/dolfinx/fem/element.py # python/dolfinx/fem/function.py # python/dolfinx/fem/petsc.py # python/dolfinx/la/superlu_dist.py # python/dolfinx/mesh.py # python/pyproject.toml
Post-merge fixups: main added a mesh argument to pack_coefficients, an interpolate_geometry function, and reshaped a few overload call sites (dofmaps as a sequence instead of a method, apply_lifting's now-arg-type instead of call-overload mismatch). Same runtime-safe/ statically-unverifiable scalar-dispatch pattern as the rest of this branch; verified mypy clean again afterwards. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| # (dependent on build configuration) | ||
| try: | ||
| from dolfinx.cpp.graph import partitioner_scotch # noqa | ||
| from dolfinx.cpp.graph import partitioner_scotch # type: ignore[attr-defined] # noqa |
| pass | ||
| try: | ||
| from dolfinx.cpp.graph import partitioner_parmetis # noqa | ||
| from dolfinx.cpp.graph import partitioner_parmetis # type: ignore[attr-defined] # noqa |
| pass | ||
| try: | ||
| from dolfinx.cpp.graph import partitioner_kahip # noqa | ||
| from dolfinx.cpp.graph import partitioner_kahip # type: ignore[attr-defined] # noqa |
Fixes Lint CI failure introduced by the main merge, which brought in the gersemi CMake formatter (replacing cmake-format). Purely cosmetic reformatting of the nanobind stub-generation additions; no logic changes.
The "Build and test" CI job runs mypy in package mode (`mypy -p dolfinx`) against a genuinely installed dolfinx with real nanobind-generated stubs, unlike the Lint job's `mypy dolfinx`, which never builds/installs dolfinx and so resolves dolfinx.cpp.* as Any via ignore_missing_imports, masking overload-resolution errors entirely. Package mode surfaced 34 real errors invisible in Lint mode: - bcs.py/assemble.py: the DirichletBC/insert_diagonal overload mismatches report as call-overload in package mode, not the arg-type code the ignores were scoped to; switch to codeless ignores since the reported code is unstable across the two modes. - io/utils.py: new arg-type error on write_function's getattr cast. - mesh.py: refine()'s partitioner annotation didn't include None, even though the docstring documents None as a valid value and callers (test_refinement.py) pass it. - test_mesh_partitioners.py: overly narrow inferred list element type broke when a ParameterSet skip-marker was appended. - test_mesh.py: partitioner_kahip/partitioner_parmetis are only present on the compiled module when built with those optional backends, which mypy can't know statically. - demo_mixed-topology.py, demo_static-condensation.py: same dtype-union-overload ambiguity pattern fixed elsewhere in this PR. Verified via a from-scratch venv replicating the CI job exactly (Python 3.12, non-editable install, real generated stubs): mypy passes in all three invocation modes (Lint's mypy dolfinx/test/demo, and package-mode -p dolfinx/test/demo), ruff check/format clean, and the affected test files pass under pytest.
The CI runner for the "Build and test" job doesn't have libscotch installed, so partitioner_scotch is absent from the compiled module there too (unlike my local reproduction, which has SCOTCH via Homebrew) -- same build-config-dependent attribute-existence issue as partitioner_kahip/partitioner_parmetis fixed in the previous commit.
Member
|
Just to note that Windows stubgen should be fixed in Basix first before we try and fix it here, we can then port what we understand to here. |
jhale
reviewed
Jul 30, 2026
| to set FALSE." | ||
| ) | ||
|
|
||
| option(ENABLE_NANOBIND_STUBGEN "Run nanobind stub generation" ON) |
Member
There was a problem hiding this comment.
Should probably make this DOLFINX_ENABLE...
| dolfinx/cpp/refinement.pyi | ||
| ) | ||
| else() | ||
| # On UNIX-like systems we can import the cpp compiled module before |
Member
There was a problem hiding this comment.
Seems like something we should ask about upstream, not very pretty.
Member
Author
There was a problem hiding this comment.
- fem/utils.py: narrow space0/space1's cpp objects via a match statement in interpolation_matrix so mypy can resolve the matching interpolation_matrix overload directly, instead of ignoring the call. Also gives callers a clear TypeError on mismatched dtypes instead of an opaque nanobind overload-resolution failure. - io/gmsh.py: read_from_msh's partitioner parameter typed its callback's 4th argument as AdjacencyList (the pure-Python wrapper, unused elsewhere in this file) instead of _AdjacencyList_int32 (the cpp type that model_to_mesh, which it delegates to, actually expects). Fixing the annotation removes the genuine type mismatch instead of suppressing it.
…ption Stub generation is not opt-out in practice (no CI job or packaging path disables it), so the option only added an untested configuration path. Run it unconditionally instead.
Points readers at the mechanism (stubgen imports MODULE and infers output location from __file__) that motivates staging cpp under a throwaway dolfinx/ package directory.
Instead of building cpp normally and symlinking it into a throwaway dolfinx/ directory post-build, set the cpp target's LIBRARY_OUTPUT_DIRECTORY to build directly into that directory. install(TARGETS cpp ...) still locates the target correctly regardless of its output directory, so nothing else needs to change. Verified with a from-scratch build: cpp links directly into dolfinx/cpp.<ext>, stub generation produces the same dolfinx.cpp.<submodule>-style cross-references as before, and `cmake --install` places the .so under dolfinx/ as expected.
garth-wells
marked this pull request as ready for review
July 31, 2026 14:26
The "AlmaLinux build and test" job has been failing deterministically on every run since the nanobind stub-generation work landed: every demo fails at import time with "ImportError: cannot import name 'cpp' from partially initialized module 'dolfinx' (most likely due to a circular import)". This job is the only CI job that installs dolfinx with `pip install -e` (editable). All non-editable installs across the rest of CI (the PETSc-enabled matrix in ccpp.yml, plus repeated local reproduction with an editable-install of this exact branch) succeed reliably. The new stub generation puts `dolfinx/cpp/*.pyi` (a directory of type stubs, matching nanobind's own convention for a compiled extension with nested submodules) directly alongside the compiled `dolfinx/cpp.<ext>` module. scikit-build-core's editable-install redirect builds a manifest that classifies each installed path as either a "wheel file" (compiled/source module) or a namespace-package search location; a `.pyi`-only directory that exactly shadows a compiled module's own name is an edge case scikit-build-core's own source comments show has caused prior classification bugs in this exact area (upstream issues #1427, #1482). This is a good fit for what we observe: the module resolves fine via the ordinary installed-path loader, but not through the editable redirect on this platform. This CI job doesn't need editable mode -- it builds once and immediately runs demos/tests against that one build, with no edit-and-rerun step in between -- so switching to a regular install sidesteps the redirect entirely rather than chasing the exact upstream classification bug.
…to garth/nanobind-autogen-stubs
Root-cause fix, replacing the earlier non-editable CI workaround (previous commit): editable installs were never actually broken by this PR's own code, but by a real bug in scikit-build-core <1.0.0's editable redirect finder. Empirically bisected locally (macOS, reproduced 100% on 0.11.0 through 0.12.2, 0/10 failures from 1.0.0 onward): the pre-1.0 redirect finder resolves a compiled module straight from its known file path via importlib.util.spec_from_file_location, without checking what else is on disk. nanobind's generated dolfinx/cpp/*.pyi stub directory (its standard convention for a compiled extension with nested submodules) sits right next to the compiled dolfinx/cpp.<ext> module, and the pre-1.0 build-time manifest scan registers dolfinx.cpp both as a "wheel file" (the .so) and, from the stub directory's __init__.pyi, as a package with its own search location -- confusing every subsequent `from dolfinx import cpp` in dolfinx/common.py. 1.0.0 resolves compiled modules through PathFinder instead, which correctly prefers the real file over the same-named stub directory regardless of the manifest ambiguity. Since this is a real upstream fix rather than a workaround, restore the RHEL/Spack CI job's editable install. That job's pinned Spack package repo only provides py-scikit-build-core up to 0.12.2 (confirmed by checking out the exact packages_ref tag), so pip-upgrade scikit-build-core to >=1.0.0 from PyPI specifically for that build step rather than relying on the Spack-provided one.
demo_axis.py reused the module-level `sys` (the stdlib module, used for sys.argv) as a local PETSc.Sys() instance; mypy forbids narrowing a name to an incompatible type within the same scope. Renamed to petsc_sys, which needs no suppression at all. fem/assemble.py's _assemble_matrix_csr had the same pattern on the `bcs` parameter, reassigning it from Sequence[DirichletBC] | None to a list of raw _cpp_object handles. Renamed to _bcs (matching the existing convention in fem/petsc.py), which resolves the [misc] redefinition error. The [arg-type] ignore on the following _cpp.fem.assemble_matrix call stays -- confirmed via mypy that it suppresses three separate, genuine dtype-Union-vs-concrete-overload mismatches unrelated to the renaming. Verified with ruff check/format and a targeted mypy run against the built stubs: demo_axis.py now has zero errors, assemble.py's remaining ignore is the minimal one needed.
- demo_pyamg.py: narrow poisson_problem's dtype parameter from npt.DTypeLike to type[np.floating] | type[np.complexfloating], matching how it's actually called. This also exposed that dirichletbc's own value type hint was too narrow -- it already handles anything with a .dtype attribute at runtime, just didn't declare it -- so widen fem/bcs.py's dirichletbc signature to include raw numpy scalars instead of reaching for a lossy .item() conversion (which would have silently upcast float32 boundary values to float64). - demo_pml.py / demo_scattering-boundary-conditions.py: scipy-stubs does support complex arguments to jv, just typed as numpy.complex128 /complex64, not builtin complex -- wrap m * alpha accordingly. This uncovered a real bug: compute_a was annotated -> float but always returns a genuinely complex Mie coefficient (callers already take np.real/np.abs of it) -- fixed to -> complex in both files. - demo_mixed-topology.py: cast hexahedron/prism's _cpp_object to CoordinateElement_float64 (both are built with the default dtype=np.float64, so this matches runtime reality) instead of ignoring the dtype-Union mismatch. This gives create_mesh's return type real precision, which surfaced two more pre-existing errors further down the same file that were previously masked by the broken overload match; added targeted ignores for those (same wrapper-Union-vs-concrete-overload pattern as elsewhere, no clean local fix available). - assemble.py: insert_diagonal was still passed the stale `bcs` name after the earlier _bcs rename, a runtime bug (TypeError) hidden by a bare `# type: ignore`; fixed to reference _bcs, with the ignore narrowed to [call-overload] to match the actual error code. Verified with ruff check/format, mypy against the built stubs, and by actually running demo_pyamg.py (all four dtypes, correct precision preserved) and demo_mixed-topology.py (runs through everything touched here; its pre-existing failure further on, unrelated to this change, reproduces identically on unmodified main).
Neither the Python nor the C++ doc comment previously explained why passing x with or without ghost entries changes what set() does. Traced the mechanism in DirichletBC.h's apply() lambda: _dofs0 always contains both owned and ghost dof indices, and the per-entry bounds check `_dofs0[i] < x.size()` is what makes an owned-only x safe (ghost indices are simply skipped) as well as a full local+ghost x (both get set). Also document that x0, when provided, must be at least as long as x -- only checked via assert in Debug/Developer builds, not a per-element bounds check like x itself.
demo_static-condensation.py was the only demo passing a raw PETSc.Vec directly to DirichletBC.set(), which only works at runtime because nanobind's ndarray caster happens to accept anything satisfying the buffer protocol -- but statically needs an npt.NDArray, and PETSc.Vec isn't typed as satisfying that anywhere. Every other demo doing the identical assemble_vector/apply_lifting/ghostUpdate/set sequence (demo_elasticity.py, demo_stokes.py) already uses b.array_w for exactly this call; demo_static-condensation.py had just missed it. Widening fem/bcs.py's DirichletBC.set signature to accept a buffer-like type was considered and rejected: the underlying nanobind binding's own generated stub types x as a concrete ndarray[float64, ...] regardless, so a Python-level widening would only relocate the mismatch rather than resolve it, and would require either a hard petsc4py dependency (which fem/bcs.py deliberately avoids) or a buffer-protocol Protocol requiring Python's 3.12+ collections.abc.Buffer (project floor is 3.11). Verified with ruff check/format and mypy against the built stubs; b remains the same PETSc.Vec object afterward (array_w is a zero-copy view), so the later solver.solve(b, ...) call is unaffected.
locate_dofs_geometrical/locate_dofs_topological's docstrings claimed that passing an iterable of function spaces returns "a 2-D array of shape (number of dofs, 2)". This is wrong: both the C++ implementation (std::array<std::vector<int32_t>, 2>) and every actual call site (test_bcs.py's dofs[0]/dofs[1] indexing) treat it as a list of one array per space. Fixed the docstrings, and split each function into @overload declarations so the return type (np.ndarray vs. list[np.ndarray]) is correctly narrowed per call site -- a plain Union return type was tried first and broke dofs= type-checking in 15 demos that pass a single FunctionSpace, since mypy can't tell from the Union alone which branch a given call site takes. That overload split then surfaced a second real bug: dirichletbc's `dofs` parameter was typed as a single ndarray only, but its C++ constructor also has a Sequence[ndarray]-accepting overload used when V is a sub-space and value's function space differs (e.g. demo_matrix-free-petsc.py, passing the dof-index pair straight from locate_dofs_topological((W.sub(0), V), ...)). Widened dofs to npt.NDArray[np.int32] | Sequence[npt.NDArray[np.int32]] and corrected the docstring accordingly. The remaining 5 ignores in this file (DirichletBC.set, and the dtype-dispatch construction in dirichletbc()) are the same wrapper-stores-a-dtype-Union-then-dispatches-at-runtime pattern seen throughout this codebase: bctype/`_value`/`self._cpp_object` are only known to be a *consistent* concrete dtype at runtime (via the cpp_types[dtype, geometry_dtype] lookup table), which mypy cannot verify statically. A typing.cast here would have to pick one of four concrete types with no static basis for which -- unlike the fixes above, there is no sound local fix without restructuring the dispatch mechanism itself, so these are left as targeted ignores. Verified with ruff check/format, mypy (-p dolfinx, test, and demo, all clean, matching CI's exact invocation), and pytest (test/unit/fem/test_bcs.py, 24/24 passing).
|
|
||
|
|
||
| @overload | ||
| def locate_dofs_geometrical(V: dolfinx.fem.FunctionSpace, marker: Callable) -> np.ndarray: ... |
| @overload | ||
| def locate_dofs_geometrical( | ||
| V: Iterable[dolfinx.fem.FunctionSpace], marker: Callable | ||
| ) -> list[np.ndarray]: ... |
| entity_dim: int, | ||
| entities: npt.NDArray[np.int32], | ||
| remote: bool = True, | ||
| ) -> np.ndarray: ... |
| entity_dim: int, | ||
| entities: npt.NDArray[np.int32], | ||
| remote: bool = True, | ||
| ) -> list[np.ndarray]: ... |
…ntains() demo_mixed-topology.py crashed on every CI run (confirmed identical on unmodified main via git stash, so unrelated to this PR's own commits): `dirichletbc(value=0.0, dofs=bcdofs, V=V_cpp)` passed a raw C++ FunctionSpace built from a raw C++ Mesh (both from the low-level dolfinx.cpp.mesh.create_mesh binding this demo uses directly, since UFL doesn't yet support mixed-topology domains). dirichletbc needs V.mesh to have a real UFL domain to build the Constant for the boundary value, but a raw cpp Mesh has no ufl_domain()/_ufl_is_terminal_. Fixed by reusing the same Mesh(mesh, domain)/FunctionSpace(...) wrapping idiom the file already uses later (line ~186) for form assembly -- picking one cell type's domain/element arbitrarily, since neither is used for anything beyond this association. fem/petsc.py's _assemble_matrix_petsc called `row_forms[0].function_spaces[0].contains(bc.function_space)`, but `.contains()`'s only overload takes a raw cpp FunctionSpace while `bc.function_space` returns the Python wrapper -- a TypeError on every block-assembled LinearProblem.solve() with a DirichletBC, breaking demo_stokes.py's nested_iterative_solver_high_level and demo_mixed-poisson.py in the PETSc-enabled CI matrix. Fixed by passing bc.function_space._cpp_object instead. (This fix already existed uncommitted in the worktree from earlier work -- committing it now since it's exactly what these two failing demos need.) Verified demo_mixed-topology.py runs to completion locally (prints "Solution vector norm ...", no exceptions) plus a clean mypy/ruff pass. petsc.py's fix verified against the exact CI traceback (same file, same line, same call site in both demo_stokes.py and demo_mixed-poisson.py); could not run it directly in this session's non-PETSc local build, but the fix is unambiguous: .contains()'s sole registered overload requires a raw cpp FunctionSpace_float64, which ._cpp_object provides and the bare wrapper does not.
Member
Author
Basix working with Windows now. |
Closed
garth-wells
added
type-hints
nanobind
finsberg
added a commit
to scientificcomputing/dolfinx-adjoint
that referenced
this pull request
Aug 3, 2026
jorgensd
added a commit
to scientificcomputing/fenicsx_ii
that referenced
this pull request
Aug 4, 2026
which now is a Python FunctionSpace, not the C++ object. Fixes various other typing issues.
finsberg
added a commit
to scientificcomputing/dolfinx-adjoint
that referenced
this pull request
Aug 4, 2026
Fix mypy issues after updates in FEniCS/dolfinx#4342
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Summary
Continues #4220 (
jhale/nanobind-autogen-stubs) with the mypy fix that PR needed: the auto-generateddolfinx.cppstubs give mypy real, precise types for the compiled extension for the first time, which surfaced ~446 errors in the "Build and test" CI job (the only mypy invocation that actually installs dolfinx before running mypy, so the only one exercising the generated stubs).cppinstead ofdolfinx.cpp, so nanobind wrote cross-submodule references likeimport cpp.la, which doesn't exist and silently resolved toAnyunder mypy — masking real errors and producing bogus "overload can never match" diagnostics. Now stages the built module under a throwawaydolfinx/package dir before invokingnanobind_add_stubso the module's real__name__isdolfinx.cpp._IntegralType/MPICommWrapperbindings that used names/const_name()values with no resolvable Python type, which broke stub cross-references.FiniteElement/AdjacencyList__eq__bindings that exposed the raw C++operator==, violatingobject.__eq__'s Liskov contract.interpolation_matrix/discrete_curl/discrete_gradientthat are always shadowed by the real ones, and a redundantread_geometry_dataregistration).nanobind/stl/map.hand.../string.hincludes that were causing stubgen to fall back to invalid raw C++ type strings.fem/petsc.py); fixed with matching# type: ignore. A handful were real bugs instead (wrong return-type annotations informs.py, dispatch-table locals missing an explicitUnionannotation).NDArrayannotations; nanobind has nostd::reference_wrappercaster; basix's C++ types aren't stub-resolvable across the package boundary), confirmed via isolated repros rather than dolfinx bugs. Suppressed per-file via a new nanobind stub pattern file (stub_patterns.txt) that injects a scoped# mypy: disable-error-code=...into just the affected generated stubs.Rebased on current
main(merge commit included) and re-verified after the merge.Test plan
mypy --config-file python/pyproject.toml -p dolfinx→ clean (matching the failing job's exact PETSc/ADIOS2-disabled config)ruff check/ruff format --checkcleanclang-format --dry-run --Werrorclean on touched C++ files-Werror) compiles cleanlyCellType.name,__eq__against unrelated objects, MPI communicator casting) pass🤖 Generated with Claude Code