Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#10526 closed defect (fixed)

plot option gridlines='minor' broken

Reported by: BugReporter Owned by: jason, was
Priority: major Milestone: sage-4.6.2
Component: graphics Keywords: plot, grid, gridlines, matplotlib
Cc: Karl-Dieter Crisman Merged in: sage-4.6.2.alpha3
Authors: Ryan Grout Reviewers: Geoffrey Ehrman, Marshall Hampton
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by Jeroen Demeyer)

plot(x,(x,0,10),gridlines='minor')

This does not implicitly add major gridlines.

Apply trac_10526_plot_gridlines.patch

Attachments (2)

10526_reviewer.patch (827 bytes) - added by Geoff Ehrman 12 years ago.
trac_10526_plot_gridlines.patch (1004 bytes) - added by Ryan 12 years ago.
update

Download all attachments as: .zip

Change History (14)

comment:1 Changed 12 years ago by Ryan

Authors: Ryan Grout
Status: newneeds_review

gridlines='minor' was not checked properly. Now plots both minor and major gridlines (because it passes 'both' to matplotlib instead of 'minor')

Changed 12 years ago by Geoff Ehrman

Attachment: 10526_reviewer.patch added

comment:2 Changed 12 years ago by Geoff Ehrman

The following is a cleaner solution, in my opinion:

             if hgridlines=="minor":
                hgridstyle['which']="both"
             if vgridlines=="minor":
                vgridstyle['which']="both

comment:3 Changed 12 years ago by Geoff Ehrman

Reviewers: Geoff Ehrman

comment:4 in reply to:  2 Changed 12 years ago by Ryan

Replying to gbe:

The following is a cleaner solution, in my opinion:

             if hgridlines=="minor":
                hgridstyle['which']="both"
             if vgridlines=="minor":
                vgridstyle['which']="both

I don't think we want to do that. It won't let us set the gridlines independently. Say we want minor horizontal ticks and major vertical ticks. We only want to set both when _both_ are 'minor'

comment:5 Changed 12 years ago by Ryan

Thanks for your suggestion Geoff :)

Changed 12 years ago by Ryan

update

comment:6 Changed 12 years ago by Karl-Dieter Crisman

Cc: Karl-Dieter Crisman added

comment:7 Changed 12 years ago by mhampton

Reviewers: Geoff EhrmanGeoff Ehrman, Marshall Hampton
Status: needs_reviewpositive_review

Looks good, positive review.

comment:8 Changed 12 years ago by Jeroen Demeyer

Milestone: sage-4.6.2

comment:9 Changed 12 years ago by Jeroen Demeyer

Description: modified (diff)

comment:10 Changed 12 years ago by Jeroen Demeyer

Description: modified (diff)

comment:11 Changed 12 years ago by Jeroen Demeyer

Merged in: sage-4.6.2.alpha3
Resolution: fixed
Status: positive_reviewclosed

comment:12 Changed 12 years ago by Jeroen Demeyer

Reviewers: Geoff Ehrman, Marshall HamptonGeoffrey Ehrman, Marshall Hampton
Note: See TracTickets for help on using tickets.