Extend mypy checks to test/#4135
Conversation
schnellerhase
force-pushed
the
schnellerhase/mypy-demo/test
branch
from
6f80a70 to
9b9ae01
Compare
schnellerhase
force-pushed
the
schnellerhase/mypy-demo/test
branch
from
c3e3fca to
adb6872
Compare
schnellerhase
force-pushed
the
schnellerhase/mypy-demo/test
branch
from
adb6872 to
a75f18d
Compare
There was a problem hiding this comment.
I'm not sure I see the convenience of having this makefile, as it assumes:
- that there is a
pythonexecutable, notpython3.
There was a problem hiding this comment.
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: |
There was a problem hiding this comment.
Should this then have a type, otherwise I don't see the point of changing it to npt.NDArray, could just be np.ndarray.
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
Doesn't this kind of show that the type hint for dirichletbc is wrong?
There was a problem hiding this comment.
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.
|
This can be merged once the |
#4154 addresses this. I do think we need a way to allow for local replication of CI failures during development. If it is not makefiles then we should think about another way to do this. |
|
Please put developer documentation in python/README.md |
|
Setup for #4216 # Note: mypy is currenlty on CI tested without petsc4py!
PYTHON = python$(py)
lint-mypy:
$(PYTHON) -m venv dolfinx-venv-mypy --system-site-packages
. dolfinx-venv-mypy/bin/activate && pip install mypy types-cffi scipy-stubs scikit-build-core nanobind
. dolfinx-venv-mypy/bin/activate && pip install git+https://github.com/fenics/ufl.git
. dolfinx-venv-mypy/bin/activate && pip install git+https://github.com/fenics/basix.git --config-settings=build-dir=build/basix
. dolfinx-venv-mypy/bin/activate && pip install git+https://github.com/fenics/ffcx.git
. dolfinx-venv-mypy/bin/activate && pip install . --no-build-isolation --config-settings=build-dir=build/dolfinx -Ccmake.build-type="Developer"
. dolfinx-venv-mypy/bin/activate && mypy -p dolfinx
. dolfinx-venv-mypy/bin/activate && mypy test
. dolfinx-venv-mypy/bin/activate && mypy demo
lint-mypy-clean:
rm -rf dolfinx-venv-mypy
lint: lint-mypy
clean: lint-mypy-clean |
jhale
changed the title
mypy checks to test/ and demo/mypy checks to test/
|
Remaining Spack error is pyamg problem, going to merge. |
jhale
dismissed
jorgensd’s stale review
Makefile removed, it is not necessary, instructions added to README.md
3 participants
python/test/dirichletbcarg -Numberis typing incompatible https://peps.python.org/pep-0484/#the-numeric-tower.TODO in future
python/demo/Edited by @jhale