Skip to content

Extend mypy checks to test/ and demo/#4135

Open
schnellerhase wants to merge 8 commits intomainfrom
schnellerhase/mypy-demo/test
Open

Extend mypy checks to test/ and demo/#4135
schnellerhase wants to merge 8 commits intomainfrom
schnellerhase/mypy-demo/test

Conversation

@schnellerhase
Copy link
Copy Markdown
Contributor

@schnellerhase schnellerhase commented Mar 20, 2026

TODO (future):

  • Extend the mypy execution to install enabled setup for generated stub checking (this is currently not simply possible due to name collision of dolfinx src dir and module name)

@schnellerhase schnellerhase force-pushed the schnellerhase/mypy-demo/test branch from 6f80a70 to 9b9ae01 Compare March 20, 2026 14:39
@schnellerhase schnellerhase self-assigned this Mar 20, 2026
@schnellerhase schnellerhase added ci Continuous Integration type-hints labels Mar 20, 2026
@schnellerhase schnellerhase force-pushed the schnellerhase/mypy-demo/test branch from c3e3fca to adb6872 Compare March 20, 2026 15:11
@schnellerhase schnellerhase mentioned this pull request Mar 24, 2026
13 tasks
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I see the convenience of having this makefile, as it assumes:

  • that there is a python executable, not python3.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added optional py variable, following https://gitlab.com/petsc/petsc/-/blob/main/src/binding/petsc4py/makefile?ref_type=heads#L9.

Invoking with make py=3 lint-mypy will now use the python3 executable. If not provided defaults to python.


# +
def nullspace_elasticty(Q: fem.FunctionSpace) -> list[np.ndarray]:
def nullspace_elasticty(Q: fem.FunctionSpace) -> npt.NDArray:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this then have a type, otherwise I don't see the point of changing it to npt.NDArray, could just be np.ndarray.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes this should hold a type, but it needs the generic type of FunctionSpace - which will be introduced after this PR in #4131. So it is a first step into the right direction, dropping the list, but not quite perfect.

dofs = locate_dofs_topological(V=V, entity_dim=fdim, entities=facets)

bc = dirichletbc(value=dtype(0), dofs=dofs, V=V)
bc = dirichletbc(value=dtype(0.0), dofs=dofs, V=V) # type: ignore
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this kind of show that the type hint for dirichletbc is wrong?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem arises from the explicit dtype() invocation:

demo/demo_pyamg.py:87: error: "dtype[Any]" not callable  [operator]

I tried multiple fixes but I am not sure how to correctly create a scalar of a certain type in a typing compliant way. Arrays work fine, but constructor of a dtype seems to be a different case.

@schnellerhase schnellerhase requested a review from jorgensd April 2, 2026 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Continuous Integration type-hints

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants