MNT: trap inappropriate use of color kwarg in scatter; closes #6266 #6267

Merged
merged 1 commit into from Apr 8, 2016

Conversation

Projects
None yet
4 participants
Owner

efiring commented Apr 3, 2016

This slightly modifies and extends the special-casing of color-related
kwargs that would otherwise be passed in to a Collection instance.
Attempts to use 'color' in place of the 'c' kwarg for color-mapping
in scatter are now trapped with a ValueError in most cases. There
are still cases that are impossible to trap: a sequence of 3 or 4
floats between 0 and 1 could be either a single color spec or a
sequence of values to be color-mapped.

efiring added the needs_review label Apr 3, 2016

@WeatherGod WeatherGod commented on an outdated diff Apr 5, 2016

lib/matplotlib/axes/_axes.py
@@ -3824,19 +3824,23 @@ def scatter(self, x, y, s=None, c=None, marker='o', cmap=None, norm=None,
# Process **kwargs to handle aliases, conflicts with explicit kwargs:
facecolors = None
- ec = kwargs.pop('edgecolor', None)
+ ec = kwargs.pop('edgecolors', None)
@WeatherGod

WeatherGod Apr 5, 2016

Member

edgecolors is already explicitly named as a keyword argument up above. It would never be in kwargs.

Member

WeatherGod commented Apr 5, 2016

besides the one nitpick, this looks fine to me.

Owner

tacaswell commented Apr 8, 2016

I agree with @WeatherGod

@efiring efiring MNT: trap inappropriate use of color kwarg in scatter; closes #6266
This slightly modifies and extends the special-casing of color-related
kwargs that would otherwise be passed in to a Collection instance.
Attempts to use 'color' in place of the 'c' kwarg for color-mapping
in scatter are now trapped with a ValueError in most cases.  There
are still cases that are impossible to trap: a sequence of 3 or 4
floats between 0 and 1 could be either a single color spec or a
sequence of values to be color-mapped.
d28c02e

@WeatherGod WeatherGod merged commit 0b6aa7b into matplotlib:master Apr 8, 2016

2 checks passed

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

mdboom removed the needs_review label Apr 8, 2016

@WeatherGod WeatherGod added a commit that referenced this pull request Apr 8, 2016

@WeatherGod WeatherGod Merge pull request #6267 from efiring/scatter-color
MNT: trap inappropriate use of color kwarg in scatter; closes #6266
1f1b8b8
Member

WeatherGod commented Apr 8, 2016

backported to v1.5.x as 1f1b8b8

@tacaswell tacaswell added a commit to tacaswell/matplotlib that referenced this pull request May 22, 2016

@WeatherGod @tacaswell WeatherGod + tacaswell Merge pull request #6267 from efiring/scatter-color
MNT: trap inappropriate use of color kwarg in scatter; closes #6266
4390bdf

efiring deleted the efiring:scatter-color branch Jun 23, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment