Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add stubs for MeshTagsMetaClass #2219

Closed
wants to merge 5 commits into from
Closed

Add stubs for MeshTagsMetaClass #2219

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e6ea2b5
Add proper ignores to mypy and add stubs to meshtags
jorgensd May 31, 2022
03e797a
Expose name property
jorgensd May 31, 2022
f86f463
Rename C++ property to avoid recursion error when adding setter in py…
jorgensd May 31, 2022
802c5b0
Use _name for C++ meshtag
jorgensd May 31, 2022
3c4a7d8
Various fixes
jorgensd May 31, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions python/demo/demo_gmsh.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@

msh.topology.create_connectivity(2, 0)
mt = meshtags_from_entities(msh, 2, create_adjacencylist(entities), values)
mt.name = "ball_d1_surface"
mt._name = "ball_d1_surface"
Copy link
Member

Choose a reason for hiding this comment

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

The underscore looks wrong in a demo - it's not idiomatic.


with XDMFFile(MPI.COMM_WORLD, "out_gmsh/mesh.xdmf", "w") as file:
file.write_mesh(msh)
Expand Down Expand Up @@ -181,7 +181,7 @@
entities, values = distribute_entity_data(msh, 2, marked_facets, facet_values)
msh.topology.create_connectivity(2, 0)
mt = meshtags_from_entities(msh, 2, create_adjacencylist(entities), values)
mt.name = "ball_d2_surface"
mt._name = "ball_d2_surface"
with XDMFFile(MPI.COMM_WORLD, "out_gmsh/mesh.xdmf", "a") as file:
file.write_mesh(msh)
msh.topology.create_connectivity(2, 3)
Expand Down Expand Up @@ -258,7 +258,7 @@
entities, values = distribute_entity_data(msh, 2, marked_facets, facet_values)
msh.topology.create_connectivity(2, 0)
mt = meshtags_from_entities(msh, 2, create_adjacencylist(entities), values)
mt.name = "hex_d2_surface"
mt._name = "hex_d2_surface"

with XDMFFile(MPI.COMM_WORLD, "out_gmsh/mesh.xdmf", "a") as file:
file.write_mesh(msh)
Expand Down
10 changes: 5 additions & 5 deletions python/dolfinx/fem/bcs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,21 +121,21 @@ class initialiser. This class is combined with different

if V is not None:
try:
super().__init__(_value, dofs, V) # type: ignore
super().__init__(_value, dofs, V) # type: ignore[call-arg]
except TypeError:
super().__init__(_value, dofs, V._cpp_object) # type: ignore
super().__init__(_value, dofs, V._cpp_object) # type: ignore[call-arg]
else:
super().__init__(_value, dofs) # type: ignore
super().__init__(_value, dofs) # type: ignore[call-arg]

@property
def g(self):
"""The boundary condition value(s)"""
return self.value # type: ignore
return self.value

@property
def function_space(self) -> dolfinx.fem.FunctionSpace:
"""The function space on which the boundary condition is defined"""
return super().function_space # type: ignore
return super().function_space # type: ignore[misc]


def dirichletbc(value: typing.Union[Function, Constant, np.ndarray],
Expand Down
10 changes: 5 additions & 5 deletions python/dolfinx/fem/forms.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def __init__(self, form, V: list[_cpp.fem.FunctionSpace], coeffs, constants,
self._ufcx_form = form
ffi = cffi.FFI()
super().__init__(ffi.cast("uintptr_t", ffi.addressof(self._ufcx_form)),
V, coeffs, constants, subdomains, mesh) # type: ignore
V, coeffs, constants, subdomains, mesh) # type: ignore[call-arg]

@property
def ufcx_form(self):
Expand All @@ -66,22 +66,22 @@ def code(self) -> str:
@property
def function_spaces(self) -> typing.List[FunctionSpace]:
"""Function spaces on which this form is defined"""
return super().function_spaces # type: ignore
return super().function_spaces # type: ignore[misc]

@property
def dtype(self) -> np.dtype:
"""dtype of this form"""
return super().dtype # type: ignore
return super().dtype # type: ignore[misc]

@property
def mesh(self) -> Mesh:
"""Mesh on which this form is defined"""
return super().mesh # type: ignore
return super().mesh # type: ignore[misc]

@property
def integral_types(self):
"""Integral types in the form"""
return super().integral_types # type: ignore
return super().integral_types # type: ignore[misc]


form_types = typing.Union[FormMetaClass, _cpp.fem.Form_float32, _cpp.fem.Form_float64,
Expand Down
4 changes: 2 additions & 2 deletions python/dolfinx/la.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,11 @@ def __init__(self, map, bs):
and not created using the class initialiser.

"""
super().__init__(map, bs) # type: ignore
super().__init__(map, bs)

@property
def array(self) -> np.ndarray:
return super().array # type: ignore
return super().array # type: ignore[misc]


def vector(map, bs=1, dtype=np.float64) -> VectorMetaClass:
Expand Down
29 changes: 28 additions & 1 deletion python/dolfinx/mesh.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ def __init__(self, mesh: Mesh, dim: int, indices: np.ndarray, values: np.ndarray
directly.

"""
super().__init__(mesh, dim, indices.astype(np.int32), values) # type: ignore
super().__init__(mesh, dim, indices.astype(np.int32), values) # type: ignore[call-arg]

def ufl_id(self) -> int:
"""Object identifier.
Expand All @@ -230,6 +230,33 @@ def ufl_id(self) -> int:
"""
return id(self)

@property
def dim(self) -> int:
"""Topological dimension of the entities"""
return super().dim # type: ignore[misc]

@property
def values(self) -> np.ndarray:
""" The values corresponding to each entity"""
return super().values # type: ignore[misc]

@property
def indices(self) -> numpy.typing.NDArray[np.int32]:
""" Entity indices (local to process)"""
return super().indices # type: ignore[misc]

@property
def name(self) -> str:
return super()._name # type: ignore[misc]

@name.setter
def name(self, name: str):
super().__setattr__("_name", name)

def find(self, value: typing.Union[int, np.int8, np.int32, np.int64, np.float64]):
""" Find all entities marked with input value"""
return super().find(value) # type:ignore[misc]


def meshtags(mesh: Mesh, dim: int, indices: np.ndarray,
values: typing.Union[np.ndarray, int, float]) -> MeshTagsMetaClass:
Expand Down
2 changes: 1 addition & 1 deletion python/dolfinx/wrappers/mesh.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ void declare_meshtags(py::module& m, std::string type)
}))
.def_property_readonly("dtype", [](const dolfinx::mesh::MeshTags<T>& self)
{ return py::dtype::of<T>(); })
.def_readwrite("name", &dolfinx::mesh::MeshTags<T>::name)
.def_readwrite("_name", &dolfinx::mesh::MeshTags<T>::name)
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't look good. We should seek a proper solution before merging.

Copy link
Sponsor Member Author

@jorgensd jorgensd May 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

You cannot have the same name for a property exposed to Python that has a getter and setter (causes infinite recursion). This would be the natural way to expose variables in a pythonic way. The main problem is that the MeshTagMetaClass is not used for all constructors of meshtags in the Python layer, thus making it very hard to use typing and mypy for external libraries.

Copy link
Member

@jhale jhale Jun 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

One option instead of retyping the entire C++ module in python would be to automatically generate stubs (pyi files) for our pybind11 module:

https://mypy.readthedocs.io/en/stable/stubgen.html

or

https://github.com/sizmailov/pybind11-stubgen

However, I couldn't get it to work, admittedly only a five minute try.

Copy link
Contributor

Choose a reason for hiding this comment

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

stubgen -p cpp in the same location of cpp.so should do it.

Copy link
Sponsor Member Author

@jorgensd jorgensd Jun 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

The main issue is that the directory of cpp.so depends on the installation process (and should preferably be executed when installing the pybind layer in setup.py). I do not think we want users to have to generate stubs manually.

Copy link
Contributor

Choose a reason for hiding this comment

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

Usually those tools produce draft stubs which often require some manual updates, I thought those would be committed. Creating them on-the-fly in setup.py is also possible, but it would introduce dependencies. Additionally, if the package is already installed stubgen -p dolfinx.cpp will find it.

Copy link
Member

@jhale jhale Jun 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@jorgensd Further to your comment #2219 (comment), do you not think that the 'fix' then would be to use the Pythonic constructors for MeshTags throughout?

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

I think that would be the correct way of fixing it. It would mean that we need to wrap up xdmf.read_meshtags
and meshtags_from_entities. I guess we would need to use the C++ copy constructor to transfer the data from C++ to Python, or do you have any other suggestion as to how we could do this in a nice way?

.def_property_readonly("dim", &dolfinx::mesh::MeshTags<T>::dim)
.def_property_readonly("mesh", &dolfinx::mesh::MeshTags<T>::mesh)
.def_property_readonly("values",
Expand Down
4 changes: 2 additions & 2 deletions python/test/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,8 @@ def _create_tempdir(request):


# Assigning a function member variables is a bit of a nasty hack
_create_tempdir._sequencenumber = defaultdict(int) # type: ignore
_create_tempdir._basepaths = set() # type: ignore
_create_tempdir._sequencenumber = defaultdict(int) # type: ignore[attr-defined]
_create_tempdir._basepaths = set() # type: ignore[attr-defined]


@pytest.fixture(scope="function")
Expand Down
5 changes: 3 additions & 2 deletions python/test/unit/fem/test_custom_assembler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,9 @@
# Get the PETSc MatSetValuesLocal function via ctypes
# ctypes does not support static types well, ignore type check errors
MatSetValues_ctypes = petsc_lib_ctypes.MatSetValuesLocal
MatSetValues_ctypes.argtypes = [ctypes.c_void_p, ctypes_index, ctypes.POINTER( # type: ignore
ctypes_index), ctypes_index, ctypes.POINTER(ctypes_index), ctypes.c_void_p, ctypes.c_int] # type: ignore
MatSetValues_ctypes.argtypes = [
ctypes.c_void_p, ctypes_index, ctypes.POINTER(ctypes_index), # type: ignore[list-item,arg-type]
ctypes_index, ctypes.POINTER(ctypes_index), ctypes.c_void_p, ctypes.c_int] # type: ignore[list-item,arg-type]
del petsc_lib_ctypes


Expand Down
4 changes: 2 additions & 2 deletions python/test/unit/io/test_xdmf_meshtags.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ def test_3d(tempdir, cell_type, encoding):
mt_lines_in = file.read_meshtags(mesh_in, "lines")
units = file.read_information("units")
assert units == "mm"
assert mt_in.name == "facets"
assert mt_lines_in.name == "lines"
assert mt_in._name == "facets"
assert mt_lines_in._name == "lines"

with XDMFFile(comm, Path(tempdir, "meshtags_3d_out.xdmf"), "w", encoding=encoding) as file:
file.write_mesh(mesh_in)
Expand Down