Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

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

Merged
QuLogic merged 1 commit into
matplotlib:mastermatplotlib/matplotlib:masterfrom
anntzer:extmodsanntzer/matplotlib:extmodsCopy head branch name to clipboard
Mar 21, 2020
Merged

In the build, declare all (compulsory) extension modules together.#14442
QuLogic merged 1 commit into
matplotlib:mastermatplotlib/matplotlib:masterfrom
anntzer:extmodsanntzer/matplotlib:extmodsCopy head branch name to clipboard

Conversation

@anntzer

@anntzer anntzer commented Jun 4, 2019

Copy link
Copy Markdown
Contributor

... 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.
Comment thread setupext.py
if platform.python_implementation().lower() == 'pypy':
ext.extra_compile_args.append('-DPYPY=1')
return ext
yield ext

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.