Markers module: PEP8 fixes and MEP10 documentation fixes #1803

Merged
merged 4 commits into from Mar 15, 2013

Projects

None yet

4 participants

Contributor
NelleV commented Mar 4, 2013

Here is a patch on PEP8 changes and MEP10 compliance of the markers module.

The markers module did not have it's own documentation page for now, as the plots using markers's style imported the docstrings and concatenated them. As we decided not to do this anymore, the module now needs it's own documentation page. This patch implements this.

Cheers,
N

@pelson pelson commented on the diff Mar 15, 2013
lib/matplotlib/markers.py
============================== ===============================================
marker description
============================== ===============================================
-%s
+"." point
+"," pixel
+"o" circle
+"v" triangle_down
+"^" triangle_up
+"<" triangle_left
+">" triangle_right
+"1" tri_down
pelson
pelson Mar 15, 2013 Member

Using your syntax I read this as 1 not '1'. As far as I can see, this makes a big difference (tri_down vs tickleft).

NelleV
NelleV Mar 15, 2013 Contributor

Do you have a better way to denote a string? I've used the same syntax as for other strings (">", "^", etc)

WeatherGod
WeatherGod Mar 15, 2013 Member

How are we indicating 'none' as opposed to the python None? I would do the
same as that.

NelleV
NelleV Mar 15, 2013 Contributor

So it is consistant with the way it currently is. I'll document everything soon, so that we have a reference document with all the details of the new documentation.

@pelson pelson commented on the diff Mar 15, 2013
lib/matplotlib/markers.py
-# special-purpose marker identifiers:
-(TICKLEFT, TICKRIGHT, TICKUP, TICKDOWN,
- CARETLEFT, CARETRIGHT, CARETUP, CARETDOWN) = range(8)
+All possible markers are defined here:
pelson
pelson Mar 15, 2013 Member

Obviously enumerating the markers here duplicates the data, which was not previously a problem. I just wanted to highlight that point - I'm fine with this repetition.

Member
pelson commented Mar 15, 2013

1 comment to resolve but this looks good. 👍
Given @mdboom was the original author (AFAIK), I'd like to let @mdboom do the actual merge.

Owner
mdboom commented Mar 15, 2013

Looks good. Merging.

@mdboom mdboom merged commit e2c092a into matplotlib:master Mar 15, 2013

1 check passed

default The Travis build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment