FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Avoid using Bbox machinery in Path.get_extents; special case polylines. by anntzer · Pull Request #17995 · matplotlib/matplotlib · GitHub

  1. matplotlib
  2. matplotlib

Repository navigation

Avoid using Bbox machinery in Path.get_extents; special case polylines. - #17995

Merged
jklymak merged 1 commit into
matplotlib:masterfrom
anntzer:bezex
Jul 21, 2020
ConversationCommits (1)ChecksFiles changed
Merged

Avoid using Bbox machinery in Path.get_extents; special case polylines.#17995
jklymak merged 1 commit into
matplotlib:masterfrom
anntzer:bezex

Conversation

anntzer commented Jul 21, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

update_from_data_xy is much slower than needed; we can use plain numpy
instead. (This speeds up #17974 another ~30-40% on top of #17994.)
For polylines, we can completely skip the bezier handling for even more
speedup. (This fixes the performance regression.)

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/next_api_changes/* if API changed in a backward-incompatible way

anntzer changed the title Avoid using Bbox machinery in Path.get_extents. Avoid using Bbox machinery in Path.get_extents; special case polylines. Jul 21, 2020
jklymak approved these changes Jul 21, 2020
View reviewed changes

jklymak left a comment

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks good to me. Any reason bbox.update_from_data_xy is so slow?

anntzer commented Jul 21, 2020

Copy link
Copy Markdown
Contributor Author

Because it constructs general path objects and then iterates over it using overly general mechanisms, I guess.

Avoid using Bbox machinery in Path.get_extents; special case polylines.
3d35d26 
update_from_data_xy is much slower than needed; we can use plain numpy
instead.
For polylines, we can completely skip the bezier handling for even more
speedup.
jklymak merged commit 80848d2 into matplotlib:master Jul 21, 2020
anntzer deleted the bezex branch July 21, 2020 19:36
QuLogic added this to the v3.4.0 milestone Jul 21, 2020

timhoffm commented Jul 21, 2020

Copy link
Copy Markdown
Member

As a performance regression fix, should this be backported to 3.3.1?

jklymak commented Jul 21, 2020

Copy link
Copy Markdown
Member

I think the regression has been there since 3.0.0, so not urgent, unless there is something more than the tight_bbox PR that added to the slowdown.

QuLogic commented Aug 4, 2020

Copy link
Copy Markdown
Member

Further discussion on #17974 says this was introduced in 3.3.0, so maybe we should backport this?

QuLogic modified the milestones: v3.4.0, v3.3.1 Aug 5, 2020

QuLogic commented Aug 5, 2020

Copy link
Copy Markdown
Member

@meeseeksdev backport to v3.3.x

meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Aug 5, 2020
Backport PR matplotlib#17995: Avoid using Bbox machinery in Path.get_…
662dc23 
…extents; special case polylines.
meeseeksmachine mentioned this pull request Aug 5, 2020
Merged
QuLogic added a commit that referenced this pull request Aug 5, 2020
Merge pull request #18183 from meeseeksmachine/auto-backport-of-pr-17…
9908136 
…995-on-v3.3.x

Backport PR #17995 on branch v3.3.x (Avoid using Bbox machinery in Path.get_extents; special case polylines.)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

dopplershift dopplershift approved these changes
jklymak jklymak approved these changes

Assignees

No one assigned

Labels

Performance

Projects

None yet

Milestone

v3.3.1

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL