Addition of RC parameters by Mrngilles · Pull Request #4218 · matplotlib/matplotlib

Mrngilles · 2015-03-13T15:19:03Z

Ended the work from PR 2702, and added other RC parameters.
What was done :

By @olgabot : Added RC to remove specific spines
(Put in its own PR) Added recursive style RC parameter: you can import recursively styles from an other stylesheet
(Removed) Made scatter, fill_between, fill_betweenx use color_cycle
Added RC to control boxplot parameters, and the associated validation functions when necessary.
Added RC to control axis grid display. You can now choose whether you want x, y, or both grid.

tacaswell · 2015-03-13T15:47:56Z

lib/matplotlib/axes/_axes.py

pep8 issue (space around ==)

tacaswell · 2015-03-13T16:03:31Z

lib/matplotlib/axes/_axes.py

👎 on scatter using color cycle. This has come up before and the consensus was that scatter should be independent of color cycle because it is as scalar mappable. One of two reasons to use scatter over a plot with no line is if you want to map the color of the markers to an array (the other is to scale the size of the markers (and maybe the rotation?)).

This is also an major API break, if we do want to make scatter sometimes hook into the color cycle it should be done on the color_overhaul branch for the 2.0 release.

Alright, I did not know about that.

tacaswell · 2015-06-17T04:28:35Z

lib/matplotlib/rcsetup.py

This should say "not a valid wisker value ['range', float, (float, float)]" or something to that effect.

Mrngilles · 2015-06-17T20:51:07Z

I'm not sure why the last Travis build failed in 2.7 without args... Any idea?
Seems to be a backend related problem... But I'm not sure how this PR is related to this

tacaswell · 2015-06-17T23:37:26Z

I kicked it to restart, smelled like a transient failure

On Wed, Jun 17, 2015, 16:51 Gilles Marin notifications@github.com wrote:

I'm not sure why the last Travis build failed in 2.7 without args... Any
idea?

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

WeatherGod · 2015-06-18T14:52:56Z

It passes now.

WeatherGod · 2015-06-19T13:56:23Z

lib/matplotlib/axes/_axes.py

this probably needs a comment explaining why this is being done to prevent future developers from "fixing" this. Also, why the "not" instead of "!="?

WeatherGod · 2015-06-22T17:06:22Z

lib/matplotlib/axes/_axes.py

If I understand this correctly, then a bad situation can arise. If someone has an rc file with a different value for boxplot.whiskers, but explicitly passes in a value for whis, then the rcfile's value takes precedence. That would be incorrect. Why not set the default value of whis to None and test for that?

I have similar concerns for bootstrap because it completely ignores the possibility that the user explicitly sets some value for it.

tacaswell · 2015-07-02T17:47:53Z

lib/matplotlib/axes/_axes.py

I think you meant to change whis=None

tacaswell · 2015-07-02T17:57:41Z

Can you add tests that for all of this? I think a single 5x5 grid png will give enough to cover all of the rcparams

Can you add the tests of the 'default' values in another PR based on current master (so that if this PR changes the defaults we will know)?

Mrngilles · 2015-07-02T18:44:17Z

You want me to write tests for all the added rc, or only the boxplot ones?
When you say a 5x5 grid, you mean a 5x5 grid of axes right?
For the test of the default values, this would mean just plotting without passing optional parameters and make sure we have the good image?

tacaswell · 2015-07-02T19:20:51Z

At least the boxplot ones, but if you want to do the other ones as well that would be awesome 👍

Yes, I mean a 5x5 grid of axes. If you can cover all of the rcparams in less that is fine too (5x5 was just the first size that popped into my head, not something carefully considered).

And yes, the idea behind the baseline test is to verify that this does not change anything unless the user explicitly asks for things to be changed (which the last round of changes to boxplot did which was less than great). Now that I type this out, can you start by checking that we do not already have this test?

Mrngilles · 2015-07-02T19:26:44Z

I checked already for the last test. There is a basic one, but it includes bootstrap and sym options. I will include a test_boxplot_no_options to the tests.
Also, using a "5x5" figure would mean testing all the parameters in a single test right? I guess I don't need to write these 30 tests functions, but just want to make sure that I only need one function testing all the parameters.
Edit: I guess that for the boxplot tests, I could make a single figure, as all parameters are different from one another... I will try it this way, and add axes if necessary

tacaswell · 2015-07-02T19:45:17Z

Yes, doing it with a single function + single image is the idea. I would
do something like

def _rctest_bxp_helper(ax, rc_dict):
    with rccontext(rc_dict):
        ax.boxplot(...)

def test_all_the_boxplot_rcparams():
    list_of_rcdicts = [ ]
    arr_of_axes = plt.subplots(5, 5).ravel()
    for ax, rc_dict in zip(arr_of_axes, list_of_rcdicts):
        _rctest_bxp_helper(ax, rc_dict)

On Thu, Jul 2, 2015 at 3:26 PM Gilles Marin notifications@github.com
wrote:

I checked already for the last test. There is a basic one, but it includes
bootstrap and sym options. I will include a test_boxplot_no_options to
the tests.
Also, using a "5x5" figure would mean testing all the parameters in a
single test right? I guess I don't need to write these 30 tests functions,
but just want to make sure that I only need one function testing all the
parameters.

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

WeatherGod · 2015-07-12T13:27:19Z

ping @Mrngilles , this needs a rebase. This also blocks #4669.

Added xtick.position and ytick.position to rcsetup.py trying to fix Travis CI failure on 2.6 Removed x-y position parameter from rc default dic Added style rcParam + stylesheet recursion scatter, fill_between and fill_betweenx use color_cycle Added rcParameters for boxplot + validations functions Added rcParameter to choose grid axis display Comment tweaks added boxplot.flierprops + line length fixes fix: unable to get substyles from path or URL removed boxplot.fliersymbol because not needed Fixed boxplot checks spine removing modified + removed : from docstring removed color_cycle from scatter, fill_between, fill_betweenx Original behaviour of axes.grid + working rc axes.grid.orientation Fixed the get_substyles function to accept dictionnaries Fix PEP8 + OrderedDict issues in Travis Modified how we get rcParams in the boxplot method tried fixing test issues + pep8 fix travis for patch_artist linestyle issue removed the style rcParam to put it in an other PR used setdefault function + validation function + Error message fixed pep8 error fixed pep8 error (again) Comments and style fixes Fixed whiskers and bootstrap rcParams use Added tests for boxplot rc parameters and fixes added spine rc parameter test added test for axes.grid.axis rc parameter + typo fix in spines test Typo fix

Mrngilles · 2015-07-12T16:55:20Z

Rebased. Hope this works, but the tests should at least be okay

Mrngilles · 2015-07-12T19:20:50Z

I guess the test had an other unexpected issue... I'm not sure if that's my code or if the problem was already there, but it seems it already happened before.

jenshnielsen · 2015-07-12T19:24:34Z

Looks like the last test failure was unrelated issue. I have restarted the test

jenshnielsen · 2015-07-12T19:46:31Z

lib/matplotlib/axes/_axes.py

I think the doc string (below) needs update to reflect these changes

jenshnielsen · 2015-07-12T19:51:03Z

It looks good to me, I think this only needs an updated docstring for boxplot to reflect the changes

ENH: Addition of RC parameters

tacaswell · 2015-07-12T22:14:28Z

@olgabot @Mrngilles Thanks!

tacaswell added the status: needs review label Mar 13, 2015

tacaswell reviewed Mar 13, 2015
View reviewed changes

lib/matplotlib/axes/_axes.py Outdated

Copy link
Copy Markdown

Member

tacaswell Mar 13, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pep8 issue (space around ==)

tacaswell added status: needs revision and removed status: needs review labels Mar 13, 2015

tacaswell added this to the next point release milestone Mar 13, 2015

tacaswell added the topic: rcparams label Mar 13, 2015

tacaswell reviewed Mar 13, 2015
View reviewed changes

tacaswell added status: needs review and removed status: needs revision labels Jun 17, 2015

tacaswell reviewed Jun 17, 2015
View reviewed changes

WeatherGod reviewed Jun 19, 2015
View reviewed changes

tacaswell mentioned this pull request Jun 22, 2015

Add spines and ticks in rcParams #2702

Closed

WeatherGod reviewed Jun 22, 2015
View reviewed changes

tacaswell reviewed Jul 2, 2015
View reviewed changes

lib/matplotlib/axes/_axes.py Outdated

Copy link
Copy Markdown

Member

tacaswell Jul 2, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you meant to change whis=None

WeatherGod mentioned this pull request Jul 12, 2015

Creation of the 'classic' matplotlib style #4669

Merged

Mrngilles force-pushed the master branch from 4c489ae to 94d044e Compare July 12, 2015 16:54

PEP8 Fix

07313b9

jenshnielsen reviewed Jul 12, 2015
View reviewed changes

lib/matplotlib/axes/_axes.py

Copy link
Copy Markdown

Member

jenshnielsen Jul 12, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the doc string (below) needs update to reflect these changes

Docstring fix

6d9c2f8

tacaswell added a commit that referenced this pull request Jul 12, 2015

Merge pull request #4218 from Mrngilles/master

ecce9a1

ENH: Addition of RC parameters

tacaswell merged commit ecce9a1 into matplotlib:master Jul 12, 2015

tacaswell removed the status: needs review label Jul 12, 2015

mdboom mentioned this pull request Nov 8, 2015

boxplot rcParams are not in matplotlibrc.template #5441

Closed

tacaswell mentioned this pull request Nov 1, 2016

Add example use of axes.spines.SIDE prop in matplotlibrc #7370

Merged

Uh oh!

Conversation

Mrngilles commented Mar 13, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mrngilles commented Jun 17, 2015

Uh oh!

tacaswell commented Jun 17, 2015

Uh oh!

WeatherGod commented Jun 18, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tacaswell commented Jul 2, 2015

Uh oh!

Mrngilles commented Jul 2, 2015

Uh oh!

tacaswell commented Jul 2, 2015

Uh oh!

Mrngilles commented Jul 2, 2015

Uh oh!

tacaswell commented Jul 2, 2015

Uh oh!

WeatherGod commented Jul 12, 2015

Uh oh!

Mrngilles commented Jul 12, 2015

Uh oh!

Mrngilles commented Jul 12, 2015

Uh oh!

jenshnielsen commented Jul 12, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jenshnielsen commented Jul 12, 2015

Uh oh!

tacaswell commented Jul 12, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants