Opened 9 years ago

Closed 9 years ago

#12581 closed defect (fixed)

Fix contour and other plot default aspect ratio

Reported by: kcrisman Owned by: jason, was
Priority: critical Milestone: sage-5.0
Component: graphics Keywords:
Cc: jason, was, benjaminfjones, mboratko Merged in: sage-5.0.beta8
Authors: Karl-Dieter Crisman Reviewers: Benjamin Jones, David Loeffler
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #9744 Stopgaps:

Status badges

Description (last modified by kcrisman)

On Feb 24, 3:04 pm, kcrisman <kcris...@gmail.com> wrote:
> Jason, what's going on with
> 
> x,y = var('x,y')
> contour_plot(x^2+y^2-2,(x,-1,1), (y,-1,1))

This was due to http://trac.sagemath.org/sage_trac/ticket/12213, and not fixed by #12222, because that only focused on shape primitives.  

The fix is most likely to add an option in the decorators of all the contour_plot.py commands to put aspect ratio back to 1 for those things.

Critical because we have in the past long discussed this; I would like to make it blocker since

This should plot concentric circles centered at the origin::

        sage: x,y = var('x,y')
        sage: contour_plot(x^2+y^2-2,(x,-1,1), (y,-1,1))

doesn't look like circles, but I suppose that they are circles...

Apply trac_12581-aspect_ratio.patch.

Attachments (1)

trac_12581-aspect_ratio.patch (1.6 KB) - added by kcrisman 9 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 9 years ago by kcrisman

  • Description modified (diff)

This might depend on #9744. I don't think it touches the same parts of contour_plot.py, but if you get a problem applying, then it does. Note that we don't need the option for implicit_plot because it always gives a region plot or a contour plot.

For reviewing; please test whether this breaks anything from #11963. I don't quite understand what the 'correct' summation behavior is yet. I assume that this works okay, though.

And a question; should it be 1.0 instead of 1?

comment:2 Changed 9 years ago by kcrisman

  • Cc benjaminfjones mboratko added

comment:3 Changed 9 years ago by benjaminfjones

  • Dependencies set to #9744
  • Reviewers set to Benjamin Jones

It looks like your patch applies over #9744 with a bit of fuzz, but it doesn't apply to a clean 5.0.beta5, so I changed the dependencies. I looked through the live docs for contour_plot with the patch applied and things look much better now. I don't see any problems. I haven't looked at whether anything from #11963 breaks yet..

FWIW, there are 6 places in the Sage library .py files where aspect_ratio=1.0 is used, and 46 places where aspect_ratio=1 is used (thank you, ack).

Changed 9 years ago by kcrisman

comment:4 Changed 9 years ago by kcrisman

  • Authors set to Karl-Dieter Crisman
  • Status changed from new to needs_review

Okay, I updated it to officially depend on #9744 without fuzz.

comment:5 Changed 9 years ago by davidloeffler

  • Reviewers changed from Benjamin Jones to Benjamin Jones, David Loeffler
  • Status changed from needs_review to positive_review

Looks good.

comment:6 Changed 9 years ago by jdemeyer

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