Opened 9 years ago

Last modified 5 years ago

#10889 needs_info defect

make text not give bounding box info if axis_coords is True

Reported by: jason Owned by: jason, was
Priority: major Milestone: sage-6.4
Component: graphics Keywords:
Cc: slabbe Merged in:
Authors: Jason Grout Reviewers: Sébastien Labbé
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

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.

Attachments (2)

trac-10889-text-axis-coords.patch (5.7 KB) - added by jason 9 years ago.
trac_10889_review-sl.patch (937 bytes) - added by slabbe 9 years ago.
Applies over the precedent patch

Download all attachments as: .zip

Change History (12)

Changed 9 years ago by jason

comment:1 Changed 9 years ago by jason

  • Authors set to Jason Grout
  • Cc slabbe added
  • Status changed from new to needs_review

comment:2 Changed 9 years ago by slabbe

  • Reviewers set to Sébastien Labbé
  • Status changed from needs_review to needs_info

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

comment:3 follow-up: Changed 9 years ago by jason

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.

comment:4 Changed 9 years ago by jason

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

Changed 9 years ago by slabbe

Applies over the precedent patch

comment:5 Changed 9 years ago by slabbe

attachment trac_10889_review-sl.patch added

... this new patch contains a commit message

comment:6 in reply to: ↑ 3 Changed 9 years ago by slabbe

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?
  1. 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?
  1. 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'.
  1. 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 ?

comment:7 Changed 6 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:8 Changed 6 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:9 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:10 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4
Note: See TracTickets for help on using tickets.