# Ticket #12222(closed defect: fixed)

Opened 17 months ago

## default aspect ratio for geometric objects

Reported by: Owned by: jason jason, was blocker sage-4.8 graphics was, kcrisman N/A Karl-Dieter Crisman Jason Grout sage-4.8.rc0

This ticket is about changing the default aspect ratio for these objects to 1.0:

• circle
• arc
• disk
• ellipse
• polygon

The idea is that when someone draws these, they want angles and circles to look correct in the plot. Changing the default aspect ratio for a circle also corrects the very first example in plot.py (which was broken by #12213)

## Change History

### comment:2 Changed 17 months ago by jason

• Status changed from new to needs_info

### comment:3 Changed 17 months ago by jason

• Description modified (diff)

### comment:4 Changed 17 months ago by novoselt

Let me repeat a suggestion from #12213: have a distinction between "user set aspect ratio 1" and "automatically set aspect ratio 1". In the first case we definitely must do what the user wants. In the second case it is reasonable to skew circles, if otherwise the plot will be totally horrible: something like having one dimension 5 times bigger than another.

If this is implemented, I would argue that the default always should be 1, except for cases when there is a good reason otherwise.

We think that if someone draws a circle, it is expected to be round, but for lines nobody cares.

However, if I draw two lines with slopes 2 and -1/2, I expect to see that they are perpendicular, and if they are not - it is the same as a skew circle.

It also seems to me that plot addition does not have to be associative, especially for 2D plots: to me left + right means "draw right on top of left" and it seemed to me that's what it does.

With this convention, it actually may make sense to inherit the aspect ratio from the left (which is likely to be more complicated, especially in "+=" operations). I unintentionally expected it when I had a complicated plot and wanted to add a couple lines to it. When the first plot is obtained from some function and it also has say, label options adjusted in a certain way, it gets especially annoying that adding a single line or a point discards all the customized look, unless I repeat all adjustments (which were hidden in the function).

### comment:5 Changed 17 months ago by jason

• Status changed from needs_info to needs_review

It sounds like a good idea to have better heuristics. I guess the tricky part is being smart enough to be nice, but not too smart that it is a guess what is going to happen. Can we open a ticket and discussion about this suggestion, since it is going far beyond the rather limited purpose of this ticket, which is to repair some common use case problems that #12213 breaks?

### comment:7 in reply to: ↑ 6 Changed 17 months ago by kcrisman

And, as I point out there, #3251.

### comment:8 Changed 17 months ago by jason

Can I ask again for a review of this conservative, but important, fix for some unintended consequences of #12213?

### comment:9 Changed 17 months ago by jason

• Description modified (diff)

### comment:10 Changed 17 months ago by kcrisman

• Priority changed from major to blocker

Considering it breaks a doctest (even if it still "passes")...

### comment:11 Changed 17 months ago by kcrisman

• Status changed from needs_review to needs_work
• Reviewers set to Karl-Dieter Crisman

In general, this is great. So almost ready for prime time.

Okay, here's something that still doesn't look good and is in the documentation already.

In plot.py:

f(x) = (x-3)*(x-5)*(x-7)+40
P = line([(2,0),(2,f(2))], color='black')
P += line([(8,0),(8,f(8))], color='black')
P += polygon([(2,0),(2,f(2))] + [(x, f(x)) for x in [2,2.1,..,8]] + [(8,0),(2,0)],  rgbcolor=(0.8,0.8,0.8))
P += text("$\\int_{a}^b f(x) dx$", (5, 20), fontsize=16, color='black')
P += plot(f, (1, 8.5), thickness=3)
P    # show the result

The polygon one needs to have something done with it, because it's overriding the 'automatic' and looks quite bad.

Hopefully this is the only one. Putting to 'needs work', but don't whip up a patch immediately until I've checked the rest.

I have to say, reading through the plot doc page with pictures is quite pleasant visually.

### comment:12 Changed 17 months ago by jason

I'm okay with setting the default for polygon to 'automatic'

### comment:13 Changed 17 months ago by kcrisman

I wasn't suggesting setting default for polygon to 'automatic', but to use it in this particular example. I think it's a little unusual - usually one would use the fill keyword, not the polygon, for this case!

A few more things:

• I don't know why, but I'm still getting aspect ratio automatic pictures in arcs even though the aspect ratio test in the documentation is passing.
• I have a feeling that complex plots should have 'automatic' as well, now, if that's possible to do easily (it may not be, and if it's not new then forget about it). Try, for instance, complex_plot(sqrt(x), (-5, 5), (-.1, .1)).
• Contour plots and vector fields seem to have 'automatic' now (try contour_plot(f, (-1, 1), (-200, 200), contours=(0.1, 1.0, 1.2, 1.4), cmap='hsv')) but density plots clearly are 1.0. I'm not sure which is better, but they should be the same, I think. I seem to recall them being 1.0 before, but of course all the examples are ones where 1.0 and automatic will look the same.

### comment:14 Changed 17 months ago by jdemeyer

Are there any plans to get this fixed soon, considering this has been made a blocker?

### comment:15 Changed 17 months ago by jason

I will work on this at the Sage days starting next Monday. Is that soon enough?

### comment:16 Changed 17 months ago by kcrisman

I would be okay with fixing the doctest in comment:11 by adding the 'automatic' in that example for the polygon (assuming we have another ticket to fix the underlying problem) and getting aspect ratio 1.0 pictures in arcs. The first is trivial, and hopefully the second wouldn't be too hard - must be a missing option decorator somewhere - maybe (?) it would be a good thing to show people at the Sage booth how we fix Sage... ?

The other things are something we'd want to think about on another ticket, as no current doctests break.

### comment:17 Changed 17 months ago by jdemeyer

Considering this is a blocker ticket, you should really aim for a minimal fix: simply fix the blocker issue, nothing more.

### comment:18 Changed 17 months ago by kcrisman

Okay, but then we still have changed a doctest (to be wrong) and comment:11 needs to be fixed. I would argue that William's ticket breaking the arcs is bad too, but I suppose I could live with this just so 4.8 can be out.

If I have enough internet today, I may fix this.

### comment:19 Changed 17 months ago by kcrisman

Okay, fixing comment:11 is easy. I cannot figure out why arcs don't work but ellipses don't; the code seems identical. I'll ask Jason later this morning.

### comment:20 Changed 17 months ago by kcrisman

Okay, Jason and I figured out that the ellipse thing is not a bug, it's me being dense. (The axes don't intersect!)

So upcoming reviewer patch gives positive review - Jason gives verbal consent after seeing the fix :)

### comment:21 Changed 17 months ago by kcrisman

• Status changed from needs_work to positive_review
• Description modified (diff)

### comment:22 Changed 17 months ago by jdemeyer

• Status changed from positive_review to closed
• Resolution set to fixed
• Merged in set to sage-4.8.rc0
Note: See TracTickets for help on using tickets.