/matplotlib

Fix #6136: Don't hardcode default scatter size #6202

Merged
merged 2 commits into from Mar 22, 2016

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
default changes rcparams
Projects
None yet
Milestone
  v2.0.0
4 participants
@mdboom @efiring @QuLogic @amueller
@mdboom
Owner

mdboom commented Mar 21, 2016

No description provided.

@mdboom mdboom Fix #6136: Don't hardcode default scatter size

All checks have passed

2 successful checks
continuous-integration/appveyor/pr — AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
13edbb5

mdboom added this to the 2.0 (style change major release) milestone Mar 21, 2016

mdboom added the needs_review label Mar 21, 2016

@QuLogic QuLogic and 1 other commented on an outdated diff Mar 22, 2016

View full outdated diff
doc/users/whats_new/style_changes.rst
@@ -28,6 +28,13 @@ Colors
- Grid lines are light grey solid 1pt lines. They are no longer dashed by
default.
+Plots
+`````
+
+- The default size of the elements in a scatter plot is now based on
+ the rcParam ``lines.markersize`` so it is consistent with ``plot(X,
+ Y, 'o')``. The old value was 20.
@QuLogic

QuLogic Mar 22, 2016

Member

What's the new value?

@mdboom

mdboom Mar 22, 2016

Owner

The new value is 36 (6 ^ 2), but it's not hardcoded -- it comes from lines.markersize.

@QuLogic QuLogic commented on the diff Mar 22, 2016

View full changes
lib/matplotlib/axes/_axes.py
@@ -3726,7 +3726,7 @@ def dopatch(xs, ys, **kwargs):
'facecolors', 'color'],
label_namer="y")
@docstring.dedent_interpd
- def scatter(self, x, y, s=20, c=None, marker='o', cmap=None, norm=None,
+ def scatter(self, x, y, s=None, c=None, marker='o', cmap=None, norm=None,
@QuLogic

QuLogic Mar 22, 2016

Member

The documentation for this function also needs updating.
@mdboom mdboom Update docs

All checks have passed

2 successful checks
continuous-integration/appveyor/pr — AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
5743e71
@mdboom
Owner

mdboom commented Mar 22, 2016

Updated for @QuLogic's comments

@efiring efiring added a commit that referenced this pull request Mar 22, 2016

@efiring efiring Merge pull request #6202 from mdboom/default-scatter-size 
Fix #6136: Don't hardcode default scatter size
 fa18cd4

@efiring efiring merged commit fa18cd4 into matplotlib:master Mar 22, 2016
2 checks passed

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

efiring removed the needs_review label Mar 22, 2016

@efiring efiring added a commit that referenced this pull request Mar 22, 2016

@efiring efiring Merge pull request #6202 from mdboom/default-scatter-size 
Fix #6136: Don't hardcode default scatter size
 302c7fd
@efiring
Owner

efiring commented Mar 22, 2016

Backported to v2.x as 302c7fd

@QuLogic QuLogic added default changes Rcparams labels Mar 22, 2016

@amueller
Contributor

amueller commented on 13edbb5 Mar 24, 2016

I don't need that right now, but maybe for consistency marker should also be an rc param?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment