Improve typing in models.em #399
Conversation
| def __call__(self, | ||
| x: Vector, | ||
| y: Vector, | ||
| three_mult: Callable[ | ||
| [int, ArrayOrArithContainerOrScalar, ArrayOrArithContainerOrScalar], | ||
| ArrayOrArithContainerOrScalar] | None = None) -> Vector: |
There was a problem hiding this comment.
This isn't correct from my reading of some of the code below (e.g. this function also takes an ObjectArray[_Dx]), but it's not too bad.
| :arg epsilon: can be a number, for a fixed material throughout the | ||
| computation domain or a :class:`~meshmode.dof_array.DOFArray` for | ||
| spatially variable material coefficients. |
There was a problem hiding this comment.
The spatially varying case doesn't seem to be tested anywhere, right? I assumed that this would be DOFArray in that case, since TimeConstantGivenFunction doesn't exist anymore.
There was a problem hiding this comment.
Nope. I'm not sure it was tested ever. It should probably be removed and bumped to a PR.
| return float(lc) * (x * y) | ||
|
|
||
| def e_curl(field: Vector) -> Vector: | ||
| return self.space_cross_e(nabla, field, three_mult=three_mult) # pyright: ignore[reportArgumentType] |
There was a problem hiding this comment.
This is complaining because that nabla is an ObjectArray[_Dx].
| . ./ci-support-v0 | ||
| build_py_project_in_conda_env | ||
| cipip install pytest modepy | ||
| cipip install pytest modepy optype |
There was a problem hiding this comment.
This might be why we couldn't match the CI? pymbolic uses it in MultiVector and the errors were all somehow connected to MultiVector.as_vector, from what I recall.
There was a problem hiding this comment.
Oh, good catch! That makes sense. I wish the error message had been more explicit...
| . ./ci-support-v0 | ||
| build_py_project_in_conda_env | ||
| cipip install pytest modepy | ||
| cipip install pytest modepy optype |
There was a problem hiding this comment.
Oh, good catch! That makes sense. I wish the error message had been more explicit...
| :arg epsilon: can be a number, for a fixed material throughout the | ||
| computation domain or a :class:`~meshmode.dof_array.DOFArray` for | ||
| spatially variable material coefficients. |
There was a problem hiding this comment.
Nope. I'm not sure it was tested ever. It should probably be removed and bumped to a PR.
|
Thx! |
2 participants
This adds more type annotations to
models.eminspired by #398.Hopefully this makes the local / CI errors match at least in this module: it was previously complaining about a few
Unknowntypes that are now known.