-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Implement line labels feature #17035
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
base: main
Are you sure you want to change the base?
Conversation
1e1ea77
to
952a9db
Compare
952a9db
to
e9a9a58
Compare
I am trying to write an image comparison test for @image_comparison(['label_lines'])
def test_label_lines():
plt.rcParams["figure.figsize"] = (5, 3)
fig, ax = plt.subplots()
ax.spines['top'].set_visible(False)
ax.spines['right'].set_visible(False)
ax.get_xaxis().tick_bottom()
ax.get_yaxis().tick_left()
plt.plot([0, 1, 2], [0, 1, 2], label="y=x")
plt.plot([0, 1, 2], [1, 1.5, 2], label="y=x/2+1")
plt.plot([0, 1, 2], [-1, 1, 3], label="y=2x-1")
ax.label_lines() I am unsure why the annotations are not being displayed in the image comparison test. What's confusing is that when I run the same code in the python REPL with Image Comparison TestREPL with
|
Because they are off the page? |
@jklymak thanks but your comment is not helpful because there is space for them to partially render like |
Clipping? Note the tests are typically run with a different style than the default for historical reasons. |
zip(handles, labels, xys)), | ||
key=by_y): | ||
data_x, data_y = xy | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain this? Why is this more complicated than putting the text at the end of each line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our main goal with this implementation is to ensure readability for the user at minimal cost for them. The given examples such as Bachelor's degrees by gender were created by individually placing text objects and using offsets so that they do not overlap. Therefore to try and automatically format the texts in such a way that there is no overlap, we decided to create this bitmap system for spacing rather than simply placing a text at the end of each line. This need for automatic spacing is exemplified by the the same graph from the feature request, where it lacks the offsets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jklymak basically, if you place the text at the end of each line. In some cases the text will overlap, consider the bachelor's degree example posted by my colleague above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jklymak for the explanation you requested:
We are placing annotations in "buckets". To do so, we sort the last data points by Y ascending and use a boolean array to keep track of which buckets have been used. If a bucket has been used, we try to place the text in the next empty bucket.
We also consider the data point density of each bucket, if there are many data points in a bucket the annotations should be centered around the bucket rather than be placed bottom-up starting from the center (due to our iteration order because of the sort)
e9a9a58
to
e97547c
Compare
@jklymak we've made changes based on your feedback, I've marked your inline comments as resolved. We also added an example file and a new entry in We are working on unit tests and will push them as soon as they are done. |
5319e90
to
c19284f
Compare
I wonder if the constrained layout engine could get repurposed to help solve the optimal text label position problem? It is already in matplotlib for other purposes. I am not familiar enough with it though to comment on whether or not it could be used. |
There is a nice tool called
The reason I'm mentioning it, is that I'm not sure if we really want such a highly specialized code for only one single purpose. |
@WeatherGod thank you for your thought, we would need more guidance to make it actionable. I am not too familiar with the internals of constrained layout, however, it seems to me that the problem constrained layout is trying to solve is a different layout problem than we are trying to solve.
|
We discussed this during the weekly call. This feature is cool, but as is, it is likely too brittle for main-library use. i.e. if we zoom, what happens? What happens if there are 300 lines? etc. We strongly suggest anyone who wants to make this a feature to first write a third-party library, get it fully functional, and if the changes are not too invasive, then feel free to promote it for the main branch. As for @WeatherGod's suggestion, you could indeed use something like |
This may also be a good candidate to write up on the blog (https://github.com/matplotlib/matplotblog). |
Thank you for the response, therefore would it be preferable to continue working on and improving our vertical text aligner, and implement it as an independent method which takes in a list of texts and aligns them without overlap using vertical spacing which is then used by label_lines. This would take the original position into account and try and place it as near as it can be to that position. Or would such a method still be too specific. |
The biggest problem is that the text become de-coupled from the lines so if the user pans / zooms the labels no longer are in the right place. The aligning / de-conflicting needs to happen every time the view is changed. We have one example of text that does this (the code in clabel for labeling contours). If you want to keep working on this the right place to start is to create a new artist class that knows about the If you use |
@jklymak with our current implementation annotations are deleted and re-added on zoom/pan. Since the line labeling routine is fast, the user experience has been smooth. We've tested it with up to 175 lines.
In the graph below, there are 175 lines. All 175 annotations don't fit on the screen, however, the annotations that show are in the correct order starting from the minimum. When zooming in, the annotations get redrawn since the line ends are more sparsely populated. Our algorithm runs in linear time with respect to the number of lines, it positions them precisely based on available space. @ImportanceOfBeingErnest demonstrated how |
cb80237
to
d8ed7f4
Compare
d8ed7f4
to
f0239ea
Compare
So, I finally got around to downloading this. Suggest the following example for testing based on @ImportanceOfBeingErnest above. import numpy as np
import matplotlib.pyplot as plt
# Create plots with pre-defined labels.
fig, ax = plt.subplots(constrained_layout=False)
x = np.linspace(0,1,40)
Nlines = 400
y = np.cumsum(np.random.randn(40,Nlines), axis=0)
labels = ["".join(np.random.choice(list("ABCDEMNOP"), size=8)) for i in range(Nlines)]
for i, label in enumerate(labels):
ax.plot(x, y[:,i], label=label)
ax.label_lines()
plt.show() It actually all works pretty well. Sure if you have 400 lines the labels all overlap, but that seems fine. If you zoom the redraw, and if you pan around they refresh. The only issue in my mind is that they are drawn in data space, so if you zoom they can go out of view. Not sure if it would't be better to just have them at x = 1 in axes co-ordinates, and just change their y value based on what lines are visible? Otherwise this looks like a useful, relatively unobtrusive, addition. |
@jklymak I think it's a good idea to move them out of the data space into the axes coordinates, it makes more sense for a label/legend anyways |
I guess you should probably update the Bachelor's degree example to use this new function now, too. |
@QuLogic will do! |
Co-authored-by: Robert Augustynowicz <robert.augustynowicz@mail.utoronto.ca> Co-authored-by: Omar Chehab <omarchehab98@gmail.com> Co-authored-by: Jinyang Hu <jinyang.hu@mail.utoronto.ca> Co-authored-by: Dennis Tismenko <dtismenkodeveloper@gmail.com> Co-authored-by: Jinming Zhang <jinming.zhang@mail.utoronto.ca> Closes: matplotlib#12939
288d4cf
to
f72a2f4
Compare
Maybe worth comparing with https://github.com/nschloe/dufte? |
Since this Pull Request has not been updated in 60 days, it has been marked "inactive." This does not mean that it will be closed, though it may be moved to a "Draft" state. This helps maintainers prioritize their reviewing efforts. You can pick the PR back up anytime - please ping us if you need a review or guidance to move the PR forward! If you do not plan on continuing the work, please let us know so that we can either find someone to take the PR over, or close it. |
Co-authored-by: Robert Augustynowicz robert.augustynowicz@mail.utoronto.ca
Co-authored-by: Jinyang Hu jinyang.hu@mail.utoronto.ca
Co-authored-by: Dennis Tismenko dtismenkodeveloper@gmail.com
Co-authored-by: Jinming Zhang jinming.zhang@mail.utoronto.ca
Closes: #12939
Firstly, thank you for taking the time out of your day to review our pull request. We appreciate your time and feedback.
PR Summary
In this pull request, we implement a feature on
Axes
calledlabel_lines
. We chose to make it's API similar tolegend
.In our implementation we took inspiration from the Bachelor's degrees example
We got the other three example data sets from ourworldindata.org/food-prices
Inspiration
line_labels
Inspiration
line_labels
Inspiration
line_labels
Inspiration
line_labels
line_labels
PR Checklist