Don't ignore figsize in df.boxplot #16445
Conversation
…w passing it as a parameter to gca().
|
Thanks @huguesv. I think the issue is somewhere around pandas/pandas/plotting/_core.py Line 2009 in 49ec31b This simplest fix would be to wrap that call in a In [8]: with plt.rc_context({'figure.figsize': figsize}):
...: ax = plt.gca()Alternatively you could modify |
|
Sorry I got confused by the diff. I see that you found the right line :) |
pandas/plotting/_core.py
Outdated
| if ax is None and len(plt.get_fignums()) > 0: | ||
| ax = _gca() | ||
| ax = _gca(figsize) |
There was a problem hiding this comment.
I would maybe revert this change, since we pass figsize through to _plot, presumably it's handled there.
There was a problem hiding this comment.
I missed that, thanks. Changing it back.
Codecov Report
@@ Coverage Diff @@
## master #16445 +/- ##
==========================================
- Coverage 90.42% 90.41% -0.01%
==========================================
Files 161 161
Lines 51023 51029 +6
==========================================
+ Hits 46138 46140 +2
- Misses 4885 4889 +4
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #16445 +/- ##
==========================================
+ Coverage 90.42% 90.42% +<.01%
==========================================
Files 161 161
Lines 51023 51027 +4
==========================================
+ Hits 46138 46142 +4
Misses 4885 4885
Continue to review full report at Codecov.
|
|
You can add a release note in |
|
Oops, changed the wrong whatsnew file, let me fix that. |
pandas/plotting/_core.py
Outdated
| @@ -49,9 +49,13 @@ def _get_standard_kind(kind): | |||
| return {'density': 'kde'}.get(kind, kind) | |||
|
|
|||
|
|
|||
| def _gca(): | |||
| def _gca(figsize=None): | |||
There was a problem hiding this comment.
Can you change _gca to take an optional dictionary? So like
def _gca(rc=None):
with plt.rc_context(rc):
return plt.gca()that way, we aren't limited to just figsize when we call gca. You'd call it like ax = _gca({'figure.figsize': figsize})
There was a problem hiding this comment.
I can do that. Note that I can't unconditionally overwrite figure.figsize, because if it's not specified, it's None, and that would overwrite the default in rcParams. That's why I have it pass in an empty dict when it's None. Anyway, I can push that one level up the call stack like you mention.
There was a problem hiding this comment.
Ah that's unfortunate. Still probably best to handle it outside.
|
Did you have a test earlier? I thought you did, but maybe not. Anyway, could you add one in |
… test for boxplot figsize.
|
Added test. |
| @slow | ||
| def test_figsize(self): | ||
| df = DataFrame(np.random.rand(10, 5), columns=['A', 'B', 'C', 'D', 'E']) | ||
| result = df.boxplot(return_type='axes', figsize=(12,8)) |
There was a problem hiding this comment.
You'll need a comma between the 12 and 8, otherwise our style cheker will complain.
| @@ -160,6 +160,13 @@ def test_boxplot_empty_column(self): | |||
| df.loc[:, 0] = np.nan | |||
| _check_plot_works(df.boxplot, return_type='axes') | |||
|
|
|||
| @slow | |||
| def test_figsize(self): | |||
| df = DataFrame(np.random.rand(10, 5), columns=['A', 'B', 'C', 'D', 'E']) | |||
There was a problem hiding this comment.
this line might be a bit longn too. flake8 pandas/tests/plotting/test_boxplot_method.py should tell you.
|
The one test failure seemed unrealted. I've restarted it to be safe though: https://travis-ci.org/pandas-dev/pandas/jobs/235323266 |
|
All tests passed! :) |
|
@huguesv thanks. Nice job! |
* Propagate the figsize via the rcParams, since matplotlib doesn't allow passing it as a parameter to gca(). * Update what's new for v0.21.0 and use rc_context() to temporarily change rcParams. * Move bug fix from 0.21.0 whatsnew to 0.20.2. * Allow passing in an rc to _gca() instead of just figsize, and added a test for boxplot figsize. * Fix style violations. (cherry picked from commit 044feb5)
* Propagate the figsize via the rcParams, since matplotlib doesn't allow passing it as a parameter to gca(). * Update what's new for v0.21.0 and use rc_context() to temporarily change rcParams. * Move bug fix from 0.21.0 whatsnew to 0.20.2. * Allow passing in an rc to _gca() instead of just figsize, and added a test for boxplot figsize. * Fix style violations. (cherry picked from commit 044feb5)
* Propagate the figsize via the rcParams, since matplotlib doesn't allow passing it as a parameter to gca(). * Update what's new for v0.21.0 and use rc_context() to temporarily change rcParams. * Move bug fix from 0.21.0 whatsnew to 0.20.2. * Allow passing in an rc to _gca() instead of just figsize, and added a test for boxplot figsize. * Fix style violations.
Work in progress from PyCon sprints
Fix for #11959
It really doesn't feel like a proper fix, but I'm not sure what a proper fix would be (without changing matplotlib). The matplotlib
gca()ends up callingfigure(), but doesn't let us pass in afigsizetogca().Any advice? FYI this works, ie it fixes the behavior for the code snippet in the bug report. It would obviously break in multithreaded scenarios, but I'm not sure if that's a consideration or not.