Skip to content

Modernize macosx backend a bit#27874

Merged
ksunden merged 1 commit intomatplotlib:mainfrom
QuLogic:modernize-macosx
Mar 13, 2024
Merged

Modernize macosx backend a bit#27874
ksunden merged 1 commit intomatplotlib:mainfrom
QuLogic:modernize-macosx

Conversation

@QuLogic
Copy link
Copy Markdown
Member

@QuLogic QuLogic commented Mar 7, 2024

PR summary

Modify PyTypeObject definitions to match the 3.9 C-embedding tutorial. Use PyDoc_STR for possible optimization purposes. Re-order fields in a bit clearer manner (object information, new+init, dealloc, repr, methods). And finally, use the new PyModule_AddType, which does what our prepare_and_add_type inline function does.

PR checklist

@QuLogic
Copy link
Copy Markdown
Member Author

QuLogic commented Mar 7, 2024

The use of .ob_base = PyVarObject_HEAD_INIT as a struct initializer only appeared in 3.12 docs. I had hoped it would still work with 3.9, but apparently not, so I'll revert that bit.

@QuLogic QuLogic force-pushed the modernize-macosx branch from a745703 to 4b00c34 Compare March 7, 2024 08:05
@QuLogic
Copy link
Copy Markdown
Member Author

QuLogic commented Mar 7, 2024

Also, forgot to mention that I think PyTypeObject.tp_name should have been the fully-qualified name, not just the extension+type, as that is what we do in the other extensions.

@QuLogic QuLogic added this to the v3.9.0 milestone Mar 7, 2024
Copy link
Copy Markdown
Contributor

@greglucas greglucas left a comment

Choose a reason for hiding this comment

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

Python docs say it should be the fully qualified name including the package so I agree with that: https://docs.python.org/3/c-api/typeobj.html#c.PyTypeObject.tp_name

This looks good to me and the reordering makes sense.

A few other minor nit cleanups if you want to add them:

  • The mplutils.h include is no longer needed either I don't think.
  • We don't have any s# variants, so I don't think we need the #define PY_SSIZE_T_CLEAN either.

Modify `PyTypeObject` definitions to match the 3.9 C-embedding tutorial.
Use `PyDoc_STR` for possible optimization purposes. Re-order fields in a
bit clearer manner (object information, new+init, dealloc, repr,
methods). And finally, use the new `PyModule_AddType`, which does what
our `prepare_and_add_type` inline function does.
@QuLogic QuLogic force-pushed the modernize-macosx branch from 4b00c34 to bffdc18 Compare March 8, 2024 10:35
@QuLogic
Copy link
Copy Markdown
Member Author

QuLogic commented Mar 8, 2024

  • The mplutils.h include is no longer needed either I don't think.

I removed this one; it was no longer needed due to dropping _prepare_and_add_type.

  • We don't have any s# variants, so I don't think we need the #define PY_SSIZE_T_CLEAN either.

Yes, it's not needed, but I'd rather keep that one in case things change in the future.

@ksunden ksunden merged commit e16967f into matplotlib:main Mar 13, 2024
@QuLogic QuLogic deleted the modernize-macosx branch March 13, 2024 22:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants