Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#5599 closed defect (fixed)

[with patch, positive review] density_plot not centered correctly

Reported by: robertwb Owned by: was
Priority: major Milestone: sage-4.0.1
Component: graphics Keywords:
Cc: wcauchois Merged in: 4.0.1.rc0
Authors: Jason Grout Reviewers: Robert Bradshaw, Bill Cauchois
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

sage: var('x,y')
(x, y)
sage: density_plot(1/(x^10+y^10), (x, -10, 10), (y, -10, 10))

clearly illustrates this problem

Attachments (1)

trac-5599-plot-center.patch (3.8 KB) - added by jason 13 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 13 years ago by jason

  • Summary changed from density_plot not centered correctly to [with patch, needs review] density_plot not centered correctly

The patch fixes the error and also fixes the same error several other places in the plotting code.

comment:2 Changed 13 years ago by jason

  • Milestone changed from sage-3.4.2 to sage-3.4.1

comment:3 Changed 13 years ago by robertwb

  • Summary changed from [with patch, needs review] density_plot not centered correctly to [with patch, positive review] density_plot not centered correctly

Yep, that fixes the issue.

comment:4 Changed 13 years ago by mabshoff

  • Summary changed from [with patch, positive review] density_plot not centered correctly to [with patch, needs rebase] density_plot not centered correctly

Unfortunately this patch has bitrotted, so please rebase against 3.4.1.rc3.

sage-3.4.1.rc3/devel/sage$ patch -p1 < trac_5599-plot-center.patch 
patching file sage/plot/contour_plot.py
Hunk #1 succeeded at 149 (offset 1 line).
Hunk #2 FAILED at 246.
Hunk #3 FAILED at 264.
Hunk #4 succeeded at 285 (offset 14 lines).
2 out of 4 hunks FAILED -- saving rejects to file sage/plot/contour_plot.py.rej
patching file sage/plot/density_plot.py
Hunk #1 succeeded at 117 with fuzz 2.

Once it is rebased the postive review can be reinstated provided the rejects are trivial to resolve.

Cheers,

Michael

Changed 13 years ago by jason

comment:5 Changed 13 years ago by jason

  • Cc wcauchois added
  • Summary changed from [with patch, needs rebase] density_plot not centered correctly to [with patch, needs review] density_plot not centered correctly

I've rebased the patch against 4.0. Bill, can you review it?

comment:6 Changed 13 years ago by jason

Or Robert, can you review it?

comment:7 Changed 13 years ago by wcauchois

  • Summary changed from [with patch, needs review] density_plot not centered correctly to [with patch, positive review] density_plot not centered correctly

REFEREE REPORT

Applies fine to Sage 4.0.rc0, and the changes look good. I tested some other plots as well, and they seemed fine. Positive review.

comment:8 Changed 13 years ago by mhansen

  • Resolution set to fixed
  • Status changed from new to closed

Merged in 4.0.1.rc0.

comment:9 Changed 13 years ago by mhansen

  • Authors set to Jason Grout
  • Merged in set to 4.0.1.rc0
  • Reviewers set to Robert Bradshaw, Bill Cauchois
Note: See TracTickets for help on using tickets.