Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Convert remaining code to pybind11#28856
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.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I would like to get this in for 3.10, so if we could get a second review, that would be great
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
This was previously checked by using an `array_view<type, ndim>`, butmoving to pybind11 we won't have that until cast to `unchecked`.
They need only be the same number of dimensions, as sometimes code does`np.atleast_3d(array)` on something empty, which inserts the 0-dimensionin a spot that messes with the expected trailing shape.
Now that everything else is using pybind11, this works without issue.
Since we're using pybind11 everywhere, it should be fine now to accessit in any header, and putting the type caster there is clearer. We don'tneed the weird macro checks to conditionally define them either.
Co-authored-by: Antony Lee <anntzer.lee@gmail.com>
This looks like some of the grayscale have picked up a off-by-one rounding issue. Seeing this on other PRs as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I am not an expert on the pybind11 changes, but I did peruse this and it all looks reasonable to a non-expert. I reran the failing 313t tests and those now pass. I believe the other failures in Azure are unrelated and failing on other tests too.
I believe that it will be easier to review incremental updates after this is in if there is anything else to follow up on.
Great work on this@QuLogic! This was no small feat.
f8cadb3 intomatplotlib:mainUh oh!
There was an error while loading.Please reload this page.
PR summary
Now that we are building with pybind11 in all extensions, we can drop/convert several items that were used across extensions. Namely, everything in
py_adaptors(which was shared between_pathand_backend_agg) can now get a type caster, all the remainingarray_viewcan be moved topy::array_t, and theClipPathconverter issue from#27011 has been fixed.Additionally, I moved the pybind11 type casters from
py_converters_11.hto the files that define the related types. This keeps things a bit more consolidated, and avoids the weird#ifdefmacro magic that was needed before.PR checklist