Skip to content

Change args of create_topology to NDArray - #4369

Open
schnellerhase wants to merge 6 commits into
mainfrom
schnellerhase/vec-to-ndarray
Open

Change args of create_topology to NDArray#4369
schnellerhase wants to merge 6 commits into
mainfrom
schnellerhase/vec-to-ndarray

Conversation

@schnellerhase

@schnellerhase schnellerhase commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Caught by mypy check on downstream usage of create_topology, while possible due to implicit conversions, we should stick to the NDArray in the interface.

Previously this implied the (non numpy compatible signature):

def create_topology(arg0: mpi4py.MPI.Comm, arg1: Sequence[CellType], arg2: Sequence[Sequence[int]], arg3: Sequence[Sequence[int]], arg4: Sequence[Sequence[int]], arg5: Sequence[int], arg6: int, /) -> Topology:
    """Create a Topology object."""

now

def create_topology(arg0: mpi4py.MPI.Comm, arg1: Sequence[CellType], arg2: Sequence[Annotated[NDArray[numpy.int64], dict(shape=(None,), order='C', writable=False)]], arg3: Sequence[Annotated[NDArray[numpy.int64], dict(shape=(None,), order='C', writable=False)]], arg4: Sequence[Annotated[NDArray[numpy.int32], dict(shape=(None,), order='C', writable=False)]], arg5: Annotated[NDArray[numpy.int64], dict(shape=(None,), order='C', writable=False)], arg6: int, /) -> Topology:
    """Create a Topology object."""

Copilot AI lite review requested due to automatic review settings August 6, 2026 07:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@schnellerhase schnellerhase added type-hints nanobind Issues related to nanobindings labels Aug 6, 2026
@schnellerhase schnellerhase self-assigned this Aug 6, 2026
Comment on lines +343 to +354
auto to_vec_of_spans
= []<typename T>(
const std::vector<nb::ndarray<T, nb::ndim<1>, nb::c_contig>>&
vec)
{
std::vector<std::span<T>> vec_span;
vec_span.reserve(vec.size());
std::ranges::transform(
vec, std::back_inserter(vec_span),
[](auto& nd) { return std::span<T>(nd.data(), nd.size()); });
return vec_span;
};

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Shall this be moved to a shared implementation in array.h? Other exports (mostly mixed topology related) have similar code paths.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copilot AI review requested due to automatic review settings August 6, 2026 07:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Comment thread python/dolfinx/wrappers/mesh.cpp Outdated
Co-authored-by: Jørgen Schartum Dokken <dokken92@gmail.com>
Copilot AI review requested due to automatic review settings August 6, 2026 08:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Comment thread python/test/unit/mesh/test_mixed_topology.py Outdated
Comment thread python/test/unit/mesh/test_mixed_topology.py Outdated
Comment thread python/test/unit/mesh/test_mixed_topology.py Outdated
Copilot AI review requested due to automatic review settings August 6, 2026 08:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings August 6, 2026 09:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings August 6, 2026 09:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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

Labels

nanobind Issues related to nanobindings type-hints

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants