| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
I admit I haven't actually figured out how the first change made the given example faster. Edit: ax.xaxis.get_ticks_position() returns "unknown" here so ax.xaxis.get_tightbbox gets called, but I have not dug any deeper... |
Sorry, something went wrong.
|
I think this would benefit from more digging: the fact that there is no measurable time difference without sharex suggests that this method ought to be fast and the real problem for sharex is deeper. |
Sorry, something went wrong.
|
More digging completed #26150 (comment) |
Sorry, something went wrong.
| xticklabel_top = any(tick.label2.get_visible() for tick in | ||
| [ax.xaxis.majorTicks[0], ax.xaxis.minorTicks[0]]) |
There was a problem hiding this comment.
Directly check the visibility of the tick label as I think that is the most relevant part from xaxis.get_ticks_position() and will prevent the unnecessary calls to xaxis.get_tightbbox mentioned at #26150 (comment). Only checking the first tick is consistent with get_ticks_position.
There is a question in my mind about what happens if the ticks are pointing outward but unlabelled, but I think that could already be an issue with the existing approach as you could have "default" position and outward ticks. Edit: the ticks themselves aren't included in get_tightbbox so actually this makes no difference.
Sorry, something went wrong.
|
I think CodeCov was telling me that we didn't have any tests with a child axes and an automatically positioned title, so I added one. |
Sorry, something went wrong.
There was a problem hiding this comment.
This seems good to me. Does it actually speed things up?
Sorry, something went wrong.
If I take this example from #26150 With main I get 477 ms ± 13.3 ms per loop (mean ± std. dev. of 7 runs, 1 loop each) with this branch 224 ms ± 6.7 ms per loop (mean ± std. dev. of 7 runs, 1 loop each) That example has no titles though, so we are now just skipping everything. If I add a title to each subplot I get 511 ms ± 9.67 ms per loop (mean ± std. dev. of 7 runs, 1 loop each) Branch 275 ms ± 6.5 ms per loop (mean ± std. dev. of 7 runs, 1 loop each) If I add a title to each subplot and move the ticklabels to the top of each subplot, so we do actually need to calculate xaxis.get_tightbbox (but now fewer times) 707 ms ± 13.7 ms per loop (mean ± std. dev. of 7 runs, 1 loop each) Branch 464 ms ± 9.57 ms per loop (mean ± std. dev. of 7 runs, 1 loop each) |
Sorry, something went wrong.
|
We have a benchmarking test suite. I wonder if these are caught by it? @QuLogic maintains that - I actually forget where it is... |
Sorry, something went wrong.
This is news to me! |
Sorry, something went wrong.
We do; it is here. However, we don't test bbox_inches='tight' or sharex, and unfortunately, I haven't run the benchmark in a long while now. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
PR summary
I used fig_many.savefig(stream, format='svg', bbox_inches='tight') and fig_many_sharex.savefig(stream, format='svg', bbox_inches='tight') from #26150 as benchmarks. fig_many.savefig ran in ~320-330 ms regardless of this change. For fig_many_sharex I got
With main:
After (1)
After (2)
PR checklist