Fix rc grid parameter inconsistency #2110

Carreau · 2013-06-02T08:09:45Z

This is a first attempt at fixing #2109.

It introduces a condition in /lib/matplotlib/axis.py:Ticks that set gridOn to the rcparam only if Tick is major, and add an rcparam axes.grid.which.

I have a few question that I'll add directly on the relevant line in the File Changed tab.

Carreau · 2013-06-02T08:16:14Z

lib/matplotlib/axis.py

+            elif (not major) and (rcParams['axes.grid.which'] in ('both','minor')):
+                gridOn = rcParams['axes.grid'] 
+            else :
+                gridOn = False 


It feel strange to read rcParams here in Ticks if it is called axes, and is already used in axes.py . Moreover, the value of gridOn seem to be passed around here and commenting all this block does indead keep the wanted behavior. I propose to change those rcparams to <x|y>tick.<major|minor>.grid = Bool to be consistent with the all the other parameters.... unless I'm missing something.

I agree that it would be nice to rename the rcparams, but for backward compatibility I'm not sure we can.

Carreau · 2013-07-06T09:07:22Z

I also added test, not sure if they are correctly written though.

Carreau · 2013-07-06T09:32:44Z

I'll still be happy to get comment on the previous question,

It feel strange to read rcParams here in Ticks if it is called axes, and is already used in axes.py . Moreover, the value of gridOn seem to be passed around here and commenting all this block does indead keep the wanted behavior. I propose to change those rcparams to <x|y>tick.<major|minor>.grid = Bool to be consistent with the all the other parameters.... unless I'm missing something.

I can also rebase to merge commit together, or shuffle them around to have test-commit before the rest.

You might want me to add a line in the change log for the new rc-param ?

NelleV · 2013-07-16T08:21:38Z

lib/matplotlib/rcsetup.py

@@ -578,6 +580,10 @@ def __call__(self, s):
    'axes.titlesize':        ['large', validate_fontsize],  # fontsize of the
                                                            # axes title
    'axes.grid':             [False, validate_bool],   # display grid or not
+    'axes.grid.which':       ['major', validate_axis_locator],# set wether the gid are by


We follow pep8 convention: please add two spaces before the comment.

NelleV · 2013-07-18T15:29:37Z

This looks good to me. 👍 for merge

mdboom · 2013-07-18T15:47:57Z

@Carreau: Would you mind rebasing this so we can hit the big green merge button?

Carreau · 2013-07-18T15:55:45Z

Yes, I will, I saw that it was conflicting. IPython 1.0.alpha first :-)

Carreau · 2013-08-06T17:49:32Z

Sorry for the time to rebase. Done now. I can't seem to install this dev version of matplotlib on the current machine I am on, so can't run tests, but will try to do tomorrow.

dmcdougall · 2013-08-18T04:08:30Z

lib/matplotlib/tests/test_axes.py

@@ -1664,6 +1664,28 @@ def make_patch_spines_invisible(ax):
    host.tick_params(axis='x', **tkw)


+def test_rcparam_grid_minor():


Can you add a @cleanup decorator to this function, please?

Carreau · 2013-08-18T12:01:28Z

Sure, @cleanup decorator added.

Fix rc grid parameter inconsistency

burnpanck · 2015-06-25T20:08:10Z

Hm. I wrote a small wrapper around rcParam allowing me to access the params using attribute syntax. Now I have a problem: Should rc.axes.grid return a the value of rcParam['axes.grid'], or should it return a proxy object containing a proxy property for rcParam['axes.grid.which']? In short: This is somewhat breaking intuition, in that the keys of rcParam are not following the rules of a python nested object anymore. Maybe we should consider changing axes.grid to axes.grid.on or something?

tacaswell · 2015-06-26T13:56:45Z

@burnpanck You should follow the discussion of moving Artists to a managed attribute scheme (ex traitlets) which may come with such tools built in.

Carreau · 2015-06-26T14:36:00Z

You should follow the discussion of moving Artists to a managed attribute scheme (ex traitlets) which may come with such tools built in.

Is that on the ML or on one of the issues ( my github issue foo only showed 2 issues not really active) ?

tacaswell · 2015-06-26T14:37:25Z

Mostly on the mailing list, @ellisonbg is involved.

Carreau · 2015-06-26T14:45:30Z

Ok, I think I found it, its the one about SciPy Sprint. Brian is not alway reliable to let the rest of the team know about things like that. I think he is often using the Royal "We" (that part of your charm @ellisonbg). I was not on the dev ML, so I cannot "respond" to the thread, but once I can, I'll share a link to my previous work on that.

tacaswell · 2015-06-26T14:51:30Z

I have linked to your previous work a couple of times already.

On Fri, Jun 26, 2015 at 10:45 AM Matthias Bussonnier <
notifications@github.com> wrote:

Ok, I think I found it, its the one about SciPy Sprint. Brian is not alway
reliable to let the rest of the team know about things like that. I think
he is often using the Royal "We" (that part of your charm @ellisonbg
https://github.com/ellisonbg). I was not on the dev ML, so I cannot
"respond" to the thread, but once I can, I'll share a link
http://carreau.github.io/posts/09-Matplotlib-And-IPython-Config.html to
my previous work on that.

—
Reply to this email directly or view it on GitHub
#2110 (comment)
.

Carreau · 2015-06-26T14:57:20Z

Ok. I'll try to attend the BoF too, if no conflicts.

burnpanck · 2015-06-29T08:46:32Z

You should follow the discussion of moving Artists to a managed attribute scheme (ex traitlets) which may come with such tools built in.

Traitlets based attributes, that's great! I'm not really actively monitoring matplotlib's development (except for reading the release notes, whenever there are), just wanted to point out that this parameter naming could conflict with exactly such concepts. Anyhow, I'm looking forward to this!

Carreau reviewed Jun 2, 2013
View reviewed changes

NelleV reviewed Jul 16, 2013
View reviewed changes

Allow to set axes.grid.which in rcparams

ea17a92

dmcdougall reviewed Aug 18, 2013
View reviewed changes

add cleanup decorator

0aae63d

dmcdougall added a commit that referenced this pull request Aug 18, 2013

Merge pull request #2110 from Carreau/fix-rc-grid

79bd660

Fix rc grid parameter inconsistency

dmcdougall merged commit 79bd660 into matplotlib:master Aug 18, 2013

dmcdougall mentioned this pull request Aug 18, 2013

rcparam['axes.grid']=True != axes.grid(True) ? #2109

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix rc grid parameter inconsistency #2110

Fix rc grid parameter inconsistency #2110

Carreau commented Jun 2, 2013

Carreau Jun 2, 2013

mdboom Jul 16, 2013

Carreau commented Jul 6, 2013

Carreau commented Jul 6, 2013

NelleV Jul 16, 2013

NelleV commented Jul 18, 2013

mdboom commented Jul 18, 2013

Carreau commented Jul 18, 2013

Carreau commented Aug 6, 2013

dmcdougall Aug 18, 2013

Carreau commented Aug 18, 2013

burnpanck commented Jun 25, 2015

tacaswell commented Jun 26, 2015

Carreau commented Jun 26, 2015

tacaswell commented Jun 26, 2015

Carreau commented Jun 26, 2015

tacaswell commented Jun 26, 2015

Carreau commented Jun 26, 2015

burnpanck commented Jun 29, 2015

		@@ -1664,6 +1664,28 @@ def make_patch_spines_invisible(ax):
		host.tick_params(axis='x', **tkw)


		def test_rcparam_grid_minor():

Fix rc grid parameter inconsistency #2110

Fix rc grid parameter inconsistency #2110

Conversation

Carreau commented Jun 2, 2013

Carreau Jun 2, 2013

Choose a reason for hiding this comment

mdboom Jul 16, 2013

Choose a reason for hiding this comment

Carreau commented Jul 6, 2013

Carreau commented Jul 6, 2013

NelleV Jul 16, 2013

Choose a reason for hiding this comment

NelleV commented Jul 18, 2013

mdboom commented Jul 18, 2013

Carreau commented Jul 18, 2013

Carreau commented Aug 6, 2013

dmcdougall Aug 18, 2013

Choose a reason for hiding this comment

Carreau commented Aug 18, 2013

burnpanck commented Jun 25, 2015

tacaswell commented Jun 26, 2015

Carreau commented Jun 26, 2015

tacaswell commented Jun 26, 2015

Carreau commented Jun 26, 2015

tacaswell commented Jun 26, 2015

Carreau commented Jun 26, 2015

burnpanck commented Jun 29, 2015