Opened 8 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)
Change History (12)
Changed 8 years ago by
comment:1 Changed 8 years ago by
- Cc slabbe added
- Status changed from new to needs_review
comment:2 Changed 8 years ago by
- Reviewers set to Sébastien Labbé
- Status changed from needs_review to needs_info
comment:3 follow-up: ↓ 6 Changed 8 years ago by
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 8 years ago by
Or probably more proper: text('A', (0.5, 2), coordinates='axes') (instead of coords)
comment:5 Changed 8 years ago by
attachment trac_10889_review-sl.patch added
... this new patch contains a commit message
comment:6 in reply to: ↑ 3 Changed 8 years ago by
OK, I am a little bit mixed up now.
- First,
axis_coords=True
is confusing to me. Without documentation, I would think thataxis_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
.
- 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?
- 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?
- I think
text('A', (0.5, 2), coordinates='axes')
is a good improvement. Do we agree thatcoordinates='axes'
is the default? Andcoordinates='figure'
is for the actualaxis_coords=True
? An other idea could be'absolute'
vs'relative'
.
- 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
- One last question. If we add the option
'coordinates'
, do we deprecateaxis_coords
?
comment:7 Changed 6 years ago by
- Milestone changed from sage-5.11 to sage-5.12
comment:8 Changed 5 years ago by
- Milestone changed from sage-6.1 to sage-6.2
comment:9 Changed 5 years ago by
- Milestone changed from sage-6.2 to sage-6.3
comment:10 Changed 5 years ago by
- Milestone changed from sage-6.3 to sage-6.4
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 :
All tests passed on
sage/plot/plot.py
andsage/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 usedaxis_coords
to place objects outside the bounding box... which was previously adapting itself.