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: |
Description (last modified by )
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...
Attachments (1)
Change History (7)
comment:1 Changed 9 years ago by
- Description modified (diff)
comment:2 Changed 9 years ago by
- Cc benjaminfjones mboratko added
comment:3 Changed 9 years ago by
- 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
comment:4 Changed 9 years ago by
- 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
- 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
- Merged in set to sage-5.0.beta8
- Resolution set to fixed
- Status changed from positive_review to closed
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 of1
?