Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

In the build, declare all (compulsory) extension modules together. #14442

Merged
merged 1 commit into from
Mar 21, 2020

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Jun 4, 2019

... in a single function.

Splitting them over multiple classes doesn't really buy much.

Also convert the LibAgg and Qhull classes to toplevel functions,
as they play a role similar to add_numpy_flags.

I plan to do the same to agg and tkagg once #13075 gets in. done.

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

... in a single function.

Splitting them over multiple classes doesn't really buy much.

Also convert the LibAgg and Qhull classes to toplevel functions,
as they play a role similar to add_numpy_flags.
sources = [
'src/_macosx.m'
]
ext = Extension('matplotlib.backends._macosx', sources)
ext.extra_link_args.extend(['-framework', 'Cocoa'])
if platform.python_implementation().lower() == 'pypy':
ext.extra_compile_args.append('-DPYPY=1')
return ext
yield ext
Copy link
Member

Choose a reason for hiding this comment

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

Maybe it's clearer to just return a 1-element list here?

Suggested change
yield ext
return [ext]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that's actually less consistent with the sole other (non-empty) implementation of get_extensions() (the main list), but can do the change if you insist.

Copy link
Member

Choose a reason for hiding this comment

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

While return specifies the return value completely, yield specifies only one element (of possibly many), so you need more context to infer the full returned result. But I don't really mind.

@QuLogic QuLogic merged commit 50959b5 into matplotlib:master Mar 21, 2020
@anntzer anntzer deleted the extmods branch March 21, 2020 23:46
@anntzer anntzer mentioned this pull request Mar 22, 2020
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants