Skip to content
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

make text not give bounding box info if axis_coords is True #10889

Open
jasongrout opened this issue Mar 8, 2011 · 9 comments
Open

make text not give bounding box info if axis_coords is True #10889

jasongrout opened this issue Mar 8, 2011 · 9 comments

Comments

@jasongrout
Copy link
Member

if axis_coords is true, it doesn't make sense for a text object to give bounding box information, since the location should be within the figure no matter what the actual coordinates are.

This also involved teaching plot.py what to do when an object returned None for their coordinates. I also fixed some of the NaN-checking code right there to use the proper functions to check for NaN and infinity.

CC: @seblabbe

Component: graphics

Author: Jason Grout

Reviewer: Sébastien Labbé

Issue created by migration from https://trac.sagemath.org/ticket/10889

@jasongrout
Copy link
Member Author

Author: Jason Grout

@jasongrout
Copy link
Member Author

comment:1

Attachment: trac-10889-text-axis-coords.patch.gz

@seblabbe
Copy link
Contributor

seblabbe commented Mar 8, 2011

comment:2

Using sage-4.6.1, the patch fixes the problem I was having (mentioned in sage-devel. I added in a new patch with the following doctest that illustrates that my problem is fixed :

sage: P = point([(2008, 167)])
sage: print P.xmin(), P.xmax(), P.ymin(), P.ymax()
2007.0 2009.0 166.0 168.0
sage: P += text('Evolution', (0.5, 0.9), axis_coords=True)
sage: print P.xmin(), P.xmax(), P.ymin(), P.ymax()
2007.0 2009.0 166.0 168.0

All tests passed on sage/plot/plot.py and sage/plot/text.py.

Before giving a positive review, I have one question. What is the intended behavior for when axis_coords is True and the coordinates are smaller than 0 or bigger than 1? Do we want to adapt the bounding box? I personnally already used axis_coords to place objects outside the bounding box... which was previously adapting itself.

sage: P += text('Evolution', (0.5, 1.4), axis_coords=True)
sage: print P.xmin(), P.xmax(), P.ymin(), P.ymax()
2007.0 2009.0 166.0 168.0

@seblabbe
Copy link
Contributor

seblabbe commented Mar 8, 2011

Reviewer: Sébastien Labbé

@jasongrout
Copy link
Member Author

comment:3

I would suggest that we add an option to use "Figure" coordinates rather than axes coordinates for placing something outside of the axes bounding box. In the code, we could easily use transFigure instead of transAxes (I think that's correct).

I think it's a bug that using axes coordinates lets you place something outside of the axes (or rather, I think it let you place something outside of the current axes, and then it maybe adjusted the axes to fit what you had??)

Maybe a more comprehensive option would be:

text('Evolution', (0.5, .2), coords='axes') or text('Evolution', (0.5, .2), coords='figure')

which would specify the transAxes or transFigure transformation to matplotlib.

@jasongrout
Copy link
Member Author

comment:4

Or probably more proper: text('A', (0.5, 2), coordinates='axes') (instead of coords)

@seblabbe
Copy link
Contributor

seblabbe commented Mar 8, 2011

Applies over the precedent patch

@seblabbe
Copy link
Contributor

seblabbe commented Mar 8, 2011

comment:5

Attachment: trac_10889_review-sl.patch.gz

attachment trac_10889_review-sl.patch added

... this new patch contains a commit message

@seblabbe
Copy link
Contributor

seblabbe commented Mar 8, 2011

comment:6

OK, I am a little bit mixed up now.

  1. First, axis_coords=True is confusing to me. Without documentation, I would think that axis_coords=True means that the coordinates are relative to the axis which is the obvious drawing default. But right now, False corresponds to the default.

I think it's a bug that using axes coordinates lets you place something outside of the axes (or rather, I think it let you place something outside of the current axes, and then it maybe adjusted the axes to fit what you had??)

I don't understand the last part of the parenthesis. I think it is a good idea to let the user place the object where he wants whatever coordinate system he uses. However, I don't know if we should or not adapt the bounding box when an object is outside for when axis_coords=True.

  1. If we choose not to adapt the bounding box for when axis_coords=True, then should we add some kind of "behavior is going to change" warnings?

  2. Here is one example. Should text('A long text', (0.98, 0.98), axis_coords=True) change the bounding box so that the long text be completely included?

  3. I think text('A', (0.5, 2), coordinates='axes') is a good improvement. Do we agree that coordinates='axes' is the default? And coordinates='figure' is for the actual axis_coords=True? An other idea could be 'absolute' vs 'relative'.

  4. Another question I answered myself. As illustrated by the following example. The real coordinates are computed only at the end and are influenced by other objects being added. The middle keeps being in the middle which is a very good thing!

sage: P = point([(100, 100), (200,200)])
sage: P += text('the middle', (0.5,0.5), axis_coords=True)
sage: P
sage: P += point([(0,0)])
sage: P
  1. One last question. If we add the option 'coordinates', do we deprecate axis_coords ?

@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@mkoeppe mkoeppe removed this from the sage-6.4 milestone Dec 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants