Derive numpy_num_face_vertices element type from member type instead of hardcoding unsigned int#412
Derive numpy_num_face_vertices element type from member type instead of hardcoding unsigned int#412
Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…gned int Co-authored-by: paulmelnikow <1487036+paulmelnikow@users.noreply.github.com>
Co-authored-by: paulmelnikow <1487036+paulmelnikow@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Updates the Python bindings to derive the NumPy array element type for mesh_t::num_face_vertices from the actual container element type, avoiding hardcoded unsigned int and reducing the risk of silent type mismatches if the C++ API changes.
Changes:
- Derive
numpy_num_face_vertices’s NumPy dtype frommesh_t::num_face_vertices::value_typeinstead of hardcodingunsigned int. - Update the
memcpysize calculation to usesizeof(T)consistently. - Extend
.gitignoreto exclude additional generated/build artifacts (CodeQL, version file, egg-info,__pycache__).
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| python/bindings.cc | Switch numpy_num_face_vertices to use container-derived element type T for the NumPy array and copy size. |
| .gitignore | Add ignores for CodeQL artifacts, generated Python version file, egg-info, and __pycache__. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| using T = typename std::remove_reference<decltype(instance.num_face_vertices)>::type::value_type; | ||
| auto ret = py::array_t<T>(instance.num_face_vertices.size()); |
There was a problem hiding this comment.
typename is not allowed here because this code is not in a template context (the type is non-dependent). This will fail to compile with errors like “typename specifier refers to non-dependent type”. Remove the leading typename from the alias. Also ensure <type_traits> is included since std::remove_reference is declared there (currently only <cstring> is included explicitly).
Hardcoding
unsigned intin two places innumpy_num_face_verticesrisks silent type mismatches ifmesh_t::num_face_verticeschanges type again.Introduces a type alias
Tderived directly from the container's declared element type:Uses
std::remove_reference<>::type(C++11) rather thanstd::remove_reference_t<>(C++14) sincesetup.pysetscxx_std=11.💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.