BUG: fix hardcoded scatter marker size issue #54204 #54304
Conversation
tarsur909
changed the title
pandas/plotting/_matplotlib/core.py
Outdated
| # This means that if rcParams are temporarily changed, | ||
| # the marker size changes as well according to mpl.rc_context(). | ||
| if mpl.rcParams["_internal.classic_mode"]: | ||
| s = 20 |
There was a problem hiding this comment.
I think the default should still be 20 but deprecated in favor of using the mpl.rcParams["lines.markersize"]
There was a problem hiding this comment.
Hey @mroeschke thanks for your review. Could you please clarify what needs to be fixed? Should I raise a deprecation warning if mpl.rcParams["_internal.classic_mode"] is True or remove this conditional altogether? If that is not what you mean, could you please elaborate?
There was a problem hiding this comment.
The conditional should probably be something like
if s is None:
warning.warning("The default of s=20 is deprecated and will change to mpl.rcParams["lines.markersize"]
" in a future version. Specify `s` to suppress this warning",
DeprecationWarning, stacklevel=find_stack_level())
There was a problem hiding this comment.
Hey @mroeschke , makes sense. Made the necessary changes.
| for method_name in dir(air_quality.plot) | ||
| if not method_name.startswith("_") | ||
| ] | ||
| import warnings |
There was a problem hiding this comment.
You can add an :okwarning: below the ipython directive instead of this
| @@ -662,3 +662,35 @@ def test_bar_plt_xaxis_intervalrange(self): | |||
| (a.get_text() == b.get_text()) | |||
| for a, b in zip(s.plot.bar().get_xticklabels(), expected) | |||
| ) | |||
|
|
|||
| @pytest.mark.filterwarnings("default") | |||
There was a problem hiding this comment.
| @@ -605,7 +605,7 @@ Plotting | |||
| ^^^^^^^^ | |||
| - Bug in :meth:`Series.plot` when invoked with ``color=None`` (:issue:`51953`) | |||
| - Fixed UserWarning in :meth:`DataFrame.plot.scatter` when invoked with ``c="b"`` (:issue:`53908`) | |||
| - | |||
| - Fixed bug in :meth:`DataFrame.plot.scatter` wherein marker size was previously hardcoded to a default value (:issue:`54204`) | |||
There was a problem hiding this comment.
Should be a note in the deprecation section now
| # the marker size changes as well according to mpl.rc_context(). | ||
| warnings.warn( | ||
| """The default of s=20 is deprecated and | ||
| has changed to mpl.rcParams['lines.markersize']. |
There was a problem hiding this comment.
- Single quotes
has changed->will be changed
| """The default of s=20 is deprecated and | ||
| has changed to mpl.rcParams['lines.markersize']. | ||
| Specify `s` to suppress this warning""", | ||
| DeprecationWarning, |
There was a problem hiding this comment.
| DeprecationWarning, | |
| FutureWarning, |
|
This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this. |
|
Thanks for the pull request, but it appears to have gone stale. If interested in continuing, please merge in the main branch, address any review comments and/or failing tests, and we can reopen. |
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.