Add support for array_t<handle> and array_t<object> #5427
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rwgk
reviewed
Nov 11, 2024
|
It looks like "CI / 🐍 3 • Clang 15 • C++20 • x64" was a spurious failure unrelated to my change. |
Yes, certainly unrelated. Similarly, "🐍 3 • Clang dev • C++11 • x64", it seems to be stuck on a wget for 2+ hours. |
This was meant to further stress-test correctness of refcount handling. All modified test functions were manually leak-checked (`while True`, top command, Python 3.12.3, Ubuntu 24.01, gcc 13.2.0).
|
Looks good, thanks for the great work on the tests! I added one commit: a1b7094 I also git merged master. Waiting for GitHub Actions to finish. |
|
The "CI / 🐍 3 • windows-latest • mingw32 (pull_request)" is also definitely unrelated. Everything else works. Merging. Thanks @MaartenBaert! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This commit adds improved support for NumPy arrays with object dtype, by allowing users to access such arrays as
array_t<handle>orarray_t<object>, which are more convenient thanarray_t<PyObject*>(which was introduced in PR #4674). In particular,array_t<object>provides automatic memory management.This feature relies on the fact that
objectandhandlehave the same memory layout asPyObject*, i.e.sizeof(object) == sizeof(handle) == sizeof(PyObject*). If this is somehow not the case, theenable_if_ttest should automatically disable this feature.I have also extended and improved the tests related to this feature with additional tests to detect memory leaks due to incorrect refcounts.
Suggested changelog entry: