Skip to content

Improve mypy coverage#992

Merged
jhale merged 15 commits intomainfrom
schnellerhase/mypy-ci
Feb 25, 2026
Merged

Improve mypy coverage#992
jhale merged 15 commits intomainfrom
schnellerhase/mypy-ci

Conversation

@schnellerhase
Copy link
Contributor

@schnellerhase schnellerhase commented Feb 24, 2026

Mypy check was only run on source code before, with the introduction of auto-generated stubs #970 this is not sufficient anymore.

Note: editable install based to avoid missing the autogenerated stubs FEniCS/dolfinx#4096 (comment).

@schnellerhase schnellerhase added CI Continuous integration type-hints Python type hinting labels Feb 24, 2026
@schnellerhase schnellerhase marked this pull request as ready for review February 24, 2026 18:04
@jhale
Copy link
Member

jhale commented Feb 25, 2026

Not sure about this; running locally mypy -p basix inside the root folder (so that we see the pyproject.toml mypy settings, but not any source folders) with a non-editable install gives:

/Users/jack.hale/fenicsx-main/lib/python3.12/site-packages/basix/_basixcpp.pyi:522: error: Overloaded function signature 2 will never be matched: signature 1's parameter type(s) are the same or broader  [overload-cannot-match]
/Users/jack.hale/fenicsx-main/lib/python3.12/site-packages/basix/finite_element.py:556: error: Incompatible return value type (got "list[list[Buffer | _SupportsArray[dtype[Any]] | _NestedSequence[_SupportsArray[dtype[Any]]] | complex | bytes | str | _NestedSequence[complex | bytes | str]]]", expected "list[list[ndarray[tuple[Any, ...], dtype[Any]]]]")  [return-value]
/Users/jack.hale/fenicsx-main/lib/python3.12/site-packages/basix/finite_element.py:565: error: Incompatible return value type (got "list[list[Buffer | _SupportsArray[dtype[Any]] | _NestedSequence[_SupportsArray[dtype[Any]]] | complex | bytes | str | _NestedSequence[complex | bytes | str]]]", expected "list[list[ndarray[tuple[Any, ...], dtype[Any]]]]")  [return-value]
Found 3 errors in 2 files (checked 13 source files)

@jhale
Copy link
Member

jhale commented Feb 25, 2026

Have one error left which annoyingly is related to the generated stubs:

@overload
def tabulate_polynomial_set(celltype: CellType, polytype: PolysetType, d: int, n: int, x: Annotated[ArrayLike, dict(dtype='float32', shape=(None, None), order='C', writable=False)]) -> Annotated[ArrayLike, dict(dtype='float32')]: ...

@overload
def tabulate_polynomial_set(celltype: CellType, polytype: PolysetType, d: int, n: int, x: Annotated[ArrayLike, dict(dtype='float64', shape=(None, None), order='C', writable=False)]) -> Annotated[ArrayLike, dict(dtype='float64')]: ...

Gives:

/Users/jack.hale/fenicsx-main/lib/python3.12/site-packages/basix/_basixcpp.pyi:522: error: Overloaded function signature 2 will never be matched: signature 1's parameter type(s) are the same or broader  [overload-cannot-match]
Found 1 error in 1 file (checked 13 source files)

I noticed this in pattern_stub.txt:

basix._basixcpp.tabulate_polynomial_set:
    def tabulate_polynomial_set(celltype: CellType, polytype: PolysetType, d: int, n: int, x: Annotated[ArrayLike, dict(dtype='float64', writable=False, shape=(None, None), order='C')]) -> Annotated[ArrayLike, dict(dtype='float64', )]: ...

Maybe it needs applying differently?

@jhale
Copy link
Member

jhale commented Feb 25, 2026

OK, it is now triggering but my fixes, which work locally, are not good for the CI. I guess there are subtle differences.

https://github.com/FEniCS/basix/actions/runs/22392500731/job/64817819539#step:8:14

@jhale
Copy link
Member

jhale commented Feb 25, 2026

There is no need for INSTALL_TIME and the .pyi file can be installed as a standard CMake target.

@jhale
Copy link
Member

jhale commented Feb 25, 2026

This nearly works. The last renaming issue I cannot get the pattern_stub.txt to match tabulate_polynomial_set at runtime, so that I can replace it.

https://nanobind.readthedocs.io/en/latest/typing.html#pattern-files

@jhale jhale added this pull request to the merge queue Feb 25, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 25, 2026
@jhale jhale added this pull request to the merge queue Feb 25, 2026
Merged via the queue into main with commit cf9c1f9 Feb 25, 2026
30 checks passed
@jhale jhale deleted the schnellerhase/mypy-ci branch February 25, 2026 16:08
@schnellerhase
Copy link
Contributor Author

What's the downside of using INSTALL_TIME in both cases and not just for windows? This gives now very different install behaviours depending on platform.

@jhale
Copy link
Member

jhale commented Feb 26, 2026

I read the manual and it says that install_time should only be used when the standard
mode doesn't work. I think the standard mode makes a lot of sense in the context of CMake - build artifacts are always built and then installed. Install time is a hack.

@jhale
Copy link
Member

jhale commented Feb 26, 2026

Win32 is always 'odd' - its philosophy on how software should be shipped and linked is fundamentally different to Unix. It may be possible to get the build mode to work on Win32 as well but I don't fancy prodding the CI for hours. Maybe I get a windows box soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI Continuous integration type-hints Python type hinting

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants