-
-
Notifications
You must be signed in to change notification settings - Fork 178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix errors and add mypy to CI #2190
Conversation
Would close #1988 once merged. |
The type checkers do not like constructs like this where we use a raise of TypeError to detect a C++ or Python object. https://github.com/FEniCS/dolfinx/blob/main/python/dolfinx/fem/bcs.py#L59 dolfinx/fem/bcs.py:62: error: "Iterable[Union[Any, FunctionSpace]]" has no attribute "_cpp_object" I would propose changing to e.g. isinstance? |
I've made some progress:
DOLFINx Python errors:
Demo errors:
|
It seems like there are projects to generate typing data from pybind11 automagically but I think it looks too complex for a few methods.
…nto jhale/mypy-fixes-and-ci
Can @jhale point me to which part of the diff requires python >= 3.8? I know that mypy wheels have dropped support for older python versions (at least on linux) so that must be part of the reason, but I am wondering if there are other changes in the diff that require python >= 3.8. I am happy to keep around a weekly CI on python 3.7 for this project by shipping a minor patch on the python version check in setup.py. Admittedly, I only run a very basic demo there, but that's representative of the basic usage of the library (e.g. for teaching). |
Some of the MetaClass stuff seems to be tricky to type, although my Python typing knowledge is admittedly limited.