patch: adding kwargs 'which' in axes.grid and 'g' toggles all tick grid lines

Hello list,

Does anybody think the proposed patches could be useful?

If somebody would benefit from them I'd be happy to place an updated version of
the patch(es) at the patch tracker.

Thanks in advance for any comments.

Kind regards,
Matthias

···

On Friday, April 23, 2010 11:08:50 am Matthias Michler wrote:

Hello list, Hello developers,

I'd like to summarize my discussion with Gökhan ("Turning off minor grids
on log scaled plot") in the last days and propose two patches.

The first patch adds the keyword argument 'which' from the axis.grid to the
method 'grid' of the Axes (axes_grid_for_major_and_minor_ticks.patch). This
allows to change the drawing of grid lines for x- and y-axis at the same
time.

Furthemore Gökhan proposed to toggle *all* (namely major and minor tick)
grid lines after pressing the key 'g'. Therefore the call
event.inaxes.grid(), which toggles only the (default) major tick grid
lines, is replaced by event.inaxes.grid(which='majorminor') (see
toggling_all_tick_grid_lines.patch).
This yields the expected behavior if e.g. all (major and minor) tick grid
lines are shown, because than toggling the grid means to remove all grid
lines. But to be honest I'm not sure the latter is the intended behavior in
all cases. For instance in the case of shown major tick lines toggling all
means turning off major tick lines and turning on minor tick lines by
pressing 'g'. This sounds a little bit crazy to me, although that's
what toggling is about.

Thanks in advance for any comments.

Kind regards,
Matthias

Hello list, Hello developers,

I'd like to summarize my discussion with Gökhan ("Turning off minor grids
on log scaled plot") in the last days and propose two patches.

The first patch adds the keyword argument 'which' from the axis.grid to the
method 'grid' of the Axes (axes_grid_for_major_and_minor_ticks.patch). This
allows to change the drawing of grid lines for x- and y-axis at the same
time.

Furthemore Gökhan proposed to toggle *all* (namely major and minor tick)
grid lines after pressing the key 'g'. Therefore the call
event.inaxes.grid(), which toggles only the (default) major tick grid
lines, is replaced by event.inaxes.grid(which='majorminor') (see
toggling_all_tick_grid_lines.patch).
This yields the expected behavior if e.g. all (major and minor) tick grid
lines are shown, because than toggling the grid means to remove all grid
lines. But to be honest I'm not sure the latter is the intended behavior in
all cases. For instance in the case of shown major tick lines toggling all
means turning off major tick lines and turning on minor tick lines by
pressing 'g'. This sounds a little bit crazy to me, although that's
what toggling is about.

I think that behavior would indeed drive the user crazy, so I would not want to commit a patch that does that.

Eric

···

On 06/08/2010 10:48 PM, Matthias Michler wrote:

On Friday, April 23, 2010 11:08:50 am Matthias Michler wrote:

Thanks in advance for any comments.

Kind regards,
Matthias

Hello list,

Does anybody think the proposed patches could be useful?

If somebody would benefit from them I'd be happy to place an updated version of
the patch(es) at the patch tracker.

Thanks in advance for any comments.

Kind regards,
Matthias

------------------------------------------------------------------------------
ThinkGeek and WIRED's GeekDad team up for the Ultimate
GeekDad Father's Day Giveaway. ONE MASSIVE PRIZE to the
lucky parental unit. See the prize list and enter to win:
http://p.sf.net/sfu/thinkgeek-promo
_______________________________________________
Matplotlib-users mailing list
Matplotlib-users@lists.sourceforge.net
matplotlib-users List Signup and Options

Hi Eric,

thanks a lot for your comment. What do you think about the first part namely
adding the kwarg 'which' from the axis method to the Axes method grid?

Kind regards,
Matthias

···

On Wednesday, June 09, 2010 11:00:31 am Eric Firing wrote:

On 06/08/2010 10:48 PM, Matthias Michler wrote:
> On Friday, April 23, 2010 11:08:50 am Matthias Michler wrote:
>> Hello list, Hello developers,
>>
>> I'd like to summarize my discussion with Gökhan ("Turning off minor
>> grids on log scaled plot") in the last days and propose two patches.
>>
>> The first patch adds the keyword argument 'which' from the axis.grid to
>> the method 'grid' of the Axes
>> (axes_grid_for_major_and_minor_ticks.patch). This allows to change the
>> drawing of grid lines for x- and y-axis at the same time.
>>
>> Furthemore Gökhan proposed to toggle *all* (namely major and minor tick)
>> grid lines after pressing the key 'g'. Therefore the call
>> event.inaxes.grid(), which toggles only the (default) major tick grid
>> lines, is replaced by event.inaxes.grid(which='majorminor') (see
>> toggling_all_tick_grid_lines.patch).
>> This yields the expected behavior if e.g. all (major and minor) tick
>> grid lines are shown, because than toggling the grid means to remove
>> all grid lines. But to be honest I'm not sure the latter is the
>> intended behavior in all cases. For instance in the case of shown major
>> tick lines toggling all means turning off major tick lines and turning
>> on minor tick lines by pressing 'g'. This sounds a little bit crazy to
>> me, although that's what toggling is about.

I think that behavior would indeed drive the user crazy, so I would not
want to commit a patch that does that.

Eric

Hello list, Hello developers,

I'd like to summarize my discussion with Gökhan ("Turning off minor
grids on log scaled plot") in the last days and propose two patches.

The first patch adds the keyword argument 'which' from the axis.grid to
the method 'grid' of the Axes
(axes_grid_for_major_and_minor_ticks.patch). This allows to change the
drawing of grid lines for x- and y-axis at the same time.

Furthemore Gökhan proposed to toggle *all* (namely major and minor tick)
grid lines after pressing the key 'g'. Therefore the call
event.inaxes.grid(), which toggles only the (default) major tick grid
lines, is replaced by event.inaxes.grid(which='majorminor') (see
toggling_all_tick_grid_lines.patch).
This yields the expected behavior if e.g. all (major and minor) tick
grid lines are shown, because than toggling the grid means to remove
all grid lines. But to be honest I'm not sure the latter is the
intended behavior in all cases. For instance in the case of shown major
tick lines toggling all means turning off major tick lines and turning
on minor tick lines by pressing 'g'. This sounds a little bit crazy to
me, although that's what toggling is about.

I think that behavior would indeed drive the user crazy, so I would not
want to commit a patch that does that.

Eric

Hi Eric,

thanks a lot for your comment. What do you think about the first part namely
adding the kwarg 'which' from the axis method to the Axes method grid?

I went ahead and did it in 8402--check to see that this is what you had in mind. The which kwarg can be 'both', 'minor', or 'major'.

Eric

···

On 06/08/2010 11:07 PM, Matthias Michler wrote:

On Wednesday, June 09, 2010 11:00:31 am Eric Firing wrote:

On 06/08/2010 10:48 PM, Matthias Michler wrote:

On Friday, April 23, 2010 11:08:50 am Matthias Michler wrote:

Kind regards,
Matthias

------------------------------------------------------------------------------
ThinkGeek and WIRED's GeekDad team up for the Ultimate
GeekDad Father's Day Giveaway. ONE MASSIVE PRIZE to the
lucky parental unit. See the prize list and enter to win:
http://p.sf.net/sfu/thinkgeek-promo
_______________________________________________
Matplotlib-users mailing list
Matplotlib-users@lists.sourceforge.net
matplotlib-users List Signup and Options

Hi Eric,

thanks a lot for this commit. This is exactly what I wanted.

There is only one thing about the new possibilities 'both', 'minor', or
'major'. The first possibility replaces 'majorminor', 'minormajor',
'some_string_including_major_and_minor', ... and I'm afraid that this could
break someone's code.

Kind regards,
Matthias

···

On Wednesday, June 09, 2010 08:00:13 pm Eric Firing wrote:

On 06/08/2010 11:07 PM, Matthias Michler wrote:
> On Wednesday, June 09, 2010 11:00:31 am Eric Firing wrote:
>> On 06/08/2010 10:48 PM, Matthias Michler wrote:
>>> On Friday, April 23, 2010 11:08:50 am Matthias Michler wrote:
>>>> Hello list, Hello developers,
>>>>
>>>> I'd like to summarize my discussion with Gökhan ("Turning off minor
>>>> grids on log scaled plot") in the last days and propose two patches.
>>>>
>>>> The first patch adds the keyword argument 'which' from the axis.grid
>>>> to the method 'grid' of the Axes
>>>> (axes_grid_for_major_and_minor_ticks.patch). This allows to change the
>>>> drawing of grid lines for x- and y-axis at the same time.
>>>>
>>>> Furthemore Gökhan proposed to toggle *all* (namely major and minor
>>>> tick) grid lines after pressing the key 'g'. Therefore the call
>>>> event.inaxes.grid(), which toggles only the (default) major tick grid
>>>> lines, is replaced by event.inaxes.grid(which='majorminor') (see
>>>> toggling_all_tick_grid_lines.patch).
>>>> This yields the expected behavior if e.g. all (major and minor) tick
>>>> grid lines are shown, because than toggling the grid means to remove
>>>> all grid lines. But to be honest I'm not sure the latter is the
>>>> intended behavior in all cases. For instance in the case of shown
>>>> major tick lines toggling all means turning off major tick lines and
>>>> turning on minor tick lines by pressing 'g'. This sounds a little bit
>>>> crazy to me, although that's what toggling is about.
>>
>> I think that behavior would indeed drive the user crazy, so I would not
>> want to commit a patch that does that.
>>
>> Eric
>
> Hi Eric,
>
> thanks a lot for your comment. What do you think about the first part
> namely adding the kwarg 'which' from the axis method to the Axes method
> grid?

I went ahead and did it in 8402--check to see that this is what you had
in mind. The which kwarg can be 'both', 'minor', or 'major'.

Hello list, Hello developers,

I'd like to summarize my discussion with Gökhan ("Turning off minor
grids on log scaled plot") in the last days and propose two patches.

The first patch adds the keyword argument 'which' from the axis.grid
to the method 'grid' of the Axes
(axes_grid_for_major_and_minor_ticks.patch). This allows to change the
drawing of grid lines for x- and y-axis at the same time.

Furthemore Gökhan proposed to toggle *all* (namely major and minor
tick) grid lines after pressing the key 'g'. Therefore the call
event.inaxes.grid(), which toggles only the (default) major tick grid
lines, is replaced by event.inaxes.grid(which='majorminor') (see
toggling_all_tick_grid_lines.patch).
This yields the expected behavior if e.g. all (major and minor) tick
grid lines are shown, because than toggling the grid means to remove
all grid lines. But to be honest I'm not sure the latter is the
intended behavior in all cases. For instance in the case of shown
major tick lines toggling all means turning off major tick lines and
turning on minor tick lines by pressing 'g'. This sounds a little bit
crazy to me, although that's what toggling is about.

I think that behavior would indeed drive the user crazy, so I would not
want to commit a patch that does that.

Eric

Hi Eric,

thanks a lot for your comment. What do you think about the first part
namely adding the kwarg 'which' from the axis method to the Axes method
grid?

I went ahead and did it in 8402--check to see that this is what you had
in mind. The which kwarg can be 'both', 'minor', or 'major'.

Hi Eric,

thanks a lot for this commit. This is exactly what I wanted.

There is only one thing about the new possibilities 'both', 'minor', or
'major'. The first possibility replaces 'majorminor', 'minormajor',
'some_string_including_major_and_minor', ... and I'm afraid that this could
break someone's code.

The docstring for Axis.grid used to say

        Set the axis grid on or off; b is a boolean. Use *which* =
         'major' | 'minor' to set the grid for major or minor ticks.

This is pretty clear--*which* is 'major' or 'minor', not a string that might contain either or both. How would a user have gotten the idea that the API included 'majorminor'? Was it documented somewhere else? Granted, because it was written using a string search to parse the kwarg, any string containing major and or minor would work, but this strikes me as a bad API, and not in keeping with the rest of mpl. Given the docstring, I suspect that the behavior was actually accidental. Both the code and the docstring have been the same since at least 2004--I got tired of tracing it back in time. In this case I am inclined to make the behavior consistent with the docstring, and then enhance it with the "both" option, as I did.

Was there a mailing list thread pointing out that "majorminor" etc would actually work, contrary to the docstring?

Eric

···

On 06/09/2010 09:15 PM, Matthias Michler wrote:

On Wednesday, June 09, 2010 08:00:13 pm Eric Firing wrote:

On 06/08/2010 11:07 PM, Matthias Michler wrote:

On Wednesday, June 09, 2010 11:00:31 am Eric Firing wrote:

On 06/08/2010 10:48 PM, Matthias Michler wrote:

On Friday, April 23, 2010 11:08:50 am Matthias Michler wrote:

Kind regards,
Matthias

------------------------------------------------------------------------------
ThinkGeek and WIRED's GeekDad team up for the Ultimate
GeekDad Father's Day Giveaway. ONE MASSIVE PRIZE to the
lucky parental unit. See the prize list and enter to win:
http://p.sf.net/sfu/thinkgeek-promo
_______________________________________________
Matplotlib-users mailing list
Matplotlib-users@lists.sourceforge.net
matplotlib-users List Signup and Options