Inline and optimize ContourLabeler.get_label_coords. #19018
Merged
Conversation
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
anntzer
force-pushed
the
glc
branch
2 times, most recently
from
ff2fe6b to
39d6d98
Compare
dopplershift
approved these changes
Nov 26, 2020
QuLogic
reviewed
Nov 27, 2020
lib/matplotlib/contour.py
Outdated
| # If all candidates are `too_close()`, just use the straightest part. | ||
| idx = adist[0] | ||
| x, y = xx[idx, hbsize], yy[idx, hbsize] | ||
| return x, y, hbsize % ctr_size |
There was a problem hiding this comment.
Is this supposed to not use idx like line 308? It may be adist[0], but that doesn't mean that idx is 0.
There was a problem hiding this comment.
indeed, reordered the logic a bit
`get_label_coords` is clearly an internal helper to `locate_label`; e.g. the `distances` parameter needs to be filled in with a very specific array that's internally computed by `locate_label`. Inline it, which also makes clearer the possibility to save some further computation for `locate_label` to return a linearized index (the part that was commented "There must be a more efficient way..."; indeed converting the array to a list of tuples to get back the index seems pretty inefficient). Also better document the implementation of `get_label_coords` (I'm not actually changing any of the code logic).
timhoffm
approved these changes
Nov 27, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
get_label_coordsis clearly an internal helper tolocate_label; e.g.the
distancesparameter needs to be filled in with a very specificarray that's internally computed by
locate_label. Inline it, whichalso makes clearer the possibility to save some further computation for
locate_labelto return a linearized index (the part that was commented"There must be a more efficient way..."; indeed converting the array to
a list of tuples to get back the index seems pretty inefficient).
Also better document the implementation of
get_label_coords(I'm notactually changing any of the code logic).
PR Summary
PR Checklist
pytestpasses).flake8on changed files to check).flake8-docstringsand runflake8 --docstring-convention=all).doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).