aspect ratio is not handled correctly in combined plots

[novoselt]

Using 4.7.2.rc0, I get the following behaviour:

p = circle((1,4), 3)
p.show() # Shows "round" circle
p += line([(3,4), (5,6)])
print p.aspect_ratio() # Outputs 1
p.set_aspect_ratio(1) # This should definitely make it 1
p.show() # Shows a distorted circle
p.show(aspect_ratio=1) # Shows "round" circle

In 4.7.1 everything behaves as expected, i.e. the first plot is not round, but two others are.

It may be related to #2100. I tried to look into it, but I don't quite understand how and where __aspect_ratio is taken into account internally. Do you guys have any clue?

[jhpalmieri]

Notice:

sage: p = circle((1,4), 3)
sage: p._extra_kwds
{}
sage: q = line([(3,4), (5,6)])
sage: q._extra_kwds
{'aspect_ratio': 'automatic'}

So when you add p and q, the _extra_kwds dicts get combined, so the automatic aspect ratio for q overrides the setting for p.

There are lots of different settings for plots, and they don't seem to be stored in consistent ways: some are in this _extra_kwds dict, some (like __aspect_ratio) are stored as attributes, and some (like "aspect ratio") are stored in more than one way, not necessarily consistently.

[novoselt]

I am fine with overwriting settings - whatever is the aspect ratio for the combination is fine (or is a matter of taste). But when I explicitly set the aspect ratio to 1 after recombination and it does not work - it is a bug!

[jhpalmieri]

Oh, I agree that it's a bug. It just might be hard to fix, or at least to fix properly.

[jason]

Thanks for diagnosing this, jhpalmieri. I think the attached patch takes care of the code changes. Some docs might need to be revised somewhere, though (like the aspect_ratio or set_aspect_ratio functions?), so I'm setting this as needs work. Anyone is welcome to help finish it!

[jason]

How come I can't go from 'new defect' directly to 'needs work'? I thought I could before.

[jdemeyer]

Milestone sage-4.7.3 deleted

[jason]

I think I covered the bases, so setting this as 'needs review'. Karl-Dieter or someone else, do you want to review this?

[jason]

Just FYI, I was testing this on a (slightly modified) 4.7.2.alpha3.

[ddrake]

This fixes the problems I recently posted about on sage-devel, and works as advertised. Doctests and documentation are good. Positive review.

[was]

I'm very, very unhappy with the impact of this patch. See #12213.

[kcrisman]

I'm very, very unhappy with the impact of this patch. See #12213.

Just a thought - maybe this could be reverted and put to 'needs work'? I guess that's up to the release manager, but seems easier than tracking down how to fix the example in #12213.

[jason]

Please don't revert this; at least, test the solution up on #12213 first. This patch fixes a rather serious problem with graphics.