Skip to content

Navigation Menu

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

Micro-optimize skew(). #22119

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

Merged
merged 1 commit into from
Jan 6, 2022
Merged

Micro-optimize skew(). #22119

merged 1 commit into from
Jan 6, 2022

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Jan 5, 2022

Similarly to what was recently done for rotate(). (skew is rather rarely used, but given that @timhoffm suggested this in #22108 (review), I may as well do it too :-))

To address @timhoffm's other comment ("Does it still make sense to keep Affine2D._mtx as a numpy array if we only do element-wise operations?"): I'm quite convinced the whole transform stack would be faster if the transformation matrix was not a numpy array (because 3x3 is a size where the numpy's overhead is generally bigger than the gains from vectorization), but changing everything at once (even better would be to move things to C, but using plain C structs (or equivalently C++ objects) to store the coefficients) would be quite a big PR. So I'm doing the easy parts first :-)

PR Summary

PR Checklist

Tests and Styling

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (install flake8-docstrings and run flake8 --docstring-convention=all).

Documentation

  • New features are documented, with examples if plot related.
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).

Similarly to what was recently done for rotate().
@timhoffm timhoffm added this to the v3.6.0 milestone Jan 6, 2022
@timhoffm
Copy link
Member

timhoffm commented Jan 6, 2022

Anybody can merge after CI pass.

@QuLogic
Copy link
Member

QuLogic commented Jan 6, 2022

To address @timhoffm's other comment ("Does it still make sense to keep Affine2D._mtx as a numpy array if we only do element-wise operations?"): I'm quite convinced the whole transform stack would be faster if the transformation matrix was not a numpy array (because 3x3 is a size where the numpy's overhead is generally bigger than the gains from vectorization), but changing everything at once (even better would be to move things to C, but using plain C structs (or equivalently C++ objects) to store the coefficients) would be quite a big PR. So I'm doing the easy parts first :-)

There exist quite a few libraries for this, but I did not get much further than skimming their docs.

@QuLogic QuLogic merged commit 9a21490 into matplotlib:main Jan 6, 2022
@anntzer anntzer deleted the skew branch January 6, 2022 06:30
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.

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