Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

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

Sign up
Appearance settings

Legend auto positioning takes text objects into account#16716

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed

Conversation

RiazCharania
Copy link

@RiazCharaniaRiazCharania commentedMar 10, 2020
edited
Loading

PR Summary

This PR addresses issue#7869 Take text objects into account in legend auto positioning. The legend's auto positioning will now take into account text objects so that they will not overlap.

PR Checklist

  • Has Pytest style unit tests
  • Code isFlake 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/api_changes.rst if API changed in a backward-incompatible way

Copy link
Member

@jklymakjklymak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

A little suspicious of this. We don’t know the text extents until a draw is done (I think?). Is there something in here that executed the draw so this works?

@tacaswelltacaswell added this to thev3.3.0 milestoneMar 10, 2020
@tacaswell
Copy link
Member

@jklymak Correct, the only way we know how big the text is going to be is to draw it (exact same problem we have with constrained layout / tight layout). The tests all have explicit.draw() calls in them.

I am a bit concerned that this PR is going to rapidly spiral out of scope.

I don't think we want to start adding a second (third, depending on if you count tight_layout and constrained_layout is different things) thing that will internally force draws as that can easily start to add multiples to the draw time.

On the other hand, we can just skip this logic when the figure isn't drawn yet, but that has the issue that saving twice will give different results and saving from the GUI will "work", but saving from a script won't.

Unless there is a third option here (:crossed_fingers: ) I don't think this will be able to go in. Despite it making things better in some ways, it will make things worse in other ways.

@jklymak
Copy link
Member

I think we could possibly get a provisional extent somehow which would be an overestimate. Might come in handy in other situations as well. Something like text_get_approx_extent that assume the text is font size high and len(str) * font size wide.

@tacaswell
Copy link
Member

Something like text_get_approx_extent that assume the text is font size high and len(str) * font size wide.

That is true,@RiazCharania@Jordan711 I suggest having a look at how we do this in the autoticker / autolabeler where we use an estimate of the tick label size to pick the number of ticks.

This may grow from "bug fix" to "new feature".

@anntzer
Copy link
Contributor

I would actually prefer avoiding overestimates, see#16153,#16171 for issues with that.
Normally text rendering is cached (per-string) so this shouldn't be too costly, although I'm not sure we also cache the extents (but I guess we could too).

@RiazCharania
Copy link
Author

quick question, running this code seems to work

import matplotlib.pyplot as plt
import matplotlib as mpl

fig, ax = plt.subplots()
plt.plot([0, 1], label="foo")
plt.text(0, 1, "Sample")
plt.text(1, 0, "Text")
plt.legend() # Create the legend

fig.savefig("legend.png")

resulting in
legend

is there a draw in there that im missing?

@jklymak
Copy link
Member

I would actually prefer avoiding overestimates, see#16153,#16171 for issues with that.

I agree with that, but in this case you are just trying to not put something near the text so an overestimate is probably OK....

@jklymak
Copy link
Member

@RiazCharania

Hmmm, interesting. Can you insert a print statement into the place where you get the extents and see what they are set to? Sometimes matplotlib is mysterious when things are drawn and its possible the legend is being updated after the text is drawn?

@RiazCharania
Copy link
Author

image

running
import matplotlib.pyplot as plt
import matplotlib as mpl

fig, ax = plt.subplots()
plt.plot([0, 1], label="foo")
plt.text(0, 1, "Sample")
plt.text(1, 0, "Text")
print('pre plt.legend()')
plt.legend() # Create the legend
print('post plt.legend()')

fig.savefig("legend.png")

with print(texts) line immediately after texts = ... in legend.py

@anntzer
Copy link
Contributor

anntzer commentedMar 10, 2020
edited
Loading

Actually (with approx. 10s thought...) I think we should just make Text.get_window_extent perform whatever drawing is necessary (creating a renderer if necessary), and then make sure the result is cached so we only do it once per string/renderer.

@jklymak
Copy link
Member

yeah, but based on the above it seems to work anyway?

@QuLogicQuLogic modified the milestones:v3.3.0,v3.4.0May 5, 2020
@QuLogicQuLogic modified the milestones:v3.4.0,v3.5.0Jan 22, 2021
@jklymakjklymak marked this pull request as draftMay 8, 2021 20:08
@QuLogicQuLogic modified the milestones:v3.5.0,v3.6.0Aug 23, 2021
@timhoffmtimhoffm modified the milestones:v3.6.0,unassignedApr 30, 2022
@story645story645 modified the milestones:unassigned,needs sortingOct 6, 2022
@github-actions
Copy link

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.

@github-actionsgithub-actionsbot added the status: inactiveMarked by the “Stale” Github Action labelJul 12, 2023
@rcomer
Copy link
Member

This was done at#27469.

@rcomerrcomer closed thisMay 5, 2025
@rcomerrcomer added status: superseded and removed status: inactiveMarked by the “Stale” Github Action labelsMay 5, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@jklymakjklymakjklymak left review comments

Assignees
No one assigned
Projects
Status: Waiting for author
Milestone
future releases
Development

Successfully merging this pull request may close these issues.

9 participants
@RiazCharania@tacaswell@jklymak@anntzer@rcomer@QuLogic@story645@timhoffm@Jordan711

[8]ページ先頭

©2009-2025 Movatter.jp