FancyArrowPatch docstring overhaul #8319

Merged
merged 5 commits into from Apr 12, 2017

Conversation

Projects

Done in Arrow drawing overhaul

5 participants
Contributor

dstansby commented Mar 17, 2017

An overhaul of the FancyArrowPatch docstring. There's a couple of kwargs I'm not too sure about so I've left them blank; suggestions welcome.

@dstansby dstansby Clean up fancyarrowpatch docstring
Docstring overhaul

Small fixes
1509c86
lib/matplotlib/patches.py
- %(AvailableConnectorstyles)s
+ Parameters
+ ----------
+ posA, posB: None, tuple, optional (default: None)
@NelleV

NelleV Mar 17, 2017

Contributor

Can you add a space before the column? Numpydoc doesn't format properly elsewise.

lib/matplotlib/patches.py
+ Parameters
+ ----------
+ posA, posB: None, tuple, optional (default: None)
+ (x,y) co-ordinates of arrow tail and arrow head respectively.
@NelleV

NelleV Mar 17, 2017

Contributor

I think it should be "coordinates".

lib/matplotlib/patches.py
- %(AvailableArrowstyles)s
+ %(AvailableArrowstyles)s
@NelleV

NelleV Mar 17, 2017

Contributor

We are working towards removing docstring interpolation. If the available arrowstyles are documented else where, can you replace this with the location of the documentation instead of the interpolation? If it isn't documented elsewhere, we can focus on this later.

@dstansby

dstansby Mar 19, 2017

Contributor

I'm not too sure how to do this - do you know of another PR or commit that has done this replacement?

@afvincent

afvincent Mar 19, 2017

Contributor

I may be wrong but I think @NelleV is referring to this kind of stuff. Unfortunately, I am not sure that the arrow styles are document elsewhere than in mpatches.ArrowStyle._style_list, which has much less information than _pprint_styles(ArrowStyle._style_list).

@@ -3990,10 +3990,10 @@ def __str__(self):
if self._posA_posB is not None:
(x1, y1), (x2, y2) = self._posA_posB
return self.__class__.__name__ \
- + "(%g,%g->%g,%g)" % (x1, y1, x2, y2)
@anntzer

anntzer Mar 18, 2017 • edited

Contributor

"{}({:g},{:g}->{:g},{:g})".format(type(self).__name__, x1, y1, x2, y2) (or similar with % formatting)

likewise below

@dstansby

dstansby Mar 19, 2017

Contributor

I'd rather not touch the actual code in this PR if that's okay?

@anntzer

anntzer Mar 19, 2017

Contributor

sure

@dstansby dstansby Small style fixes
42f7bc4

dstansby added to Current TODO in Arrow drawing overhaul Mar 19, 2017

@NelleV

NelleV approved these changes Mar 19, 2017

LGTM 👍

NelleV changed the title from FancyArrowPatch docstring overhaul to [MRG+1] FancyArrowPatch docstring overhaul Mar 19, 2017

lib/matplotlib/patches.py
**kwargs):
"""
- If *posA* and *posB* is given, a path connecting two point are
- created according to the connectionstyle. The path will be
+ If *posA* and *posB* are given, a path connecting two points are
@anntzer

anntzer Mar 20, 2017

Contributor

... the two points is created.

lib/matplotlib/patches.py
+ shrinkA, shrinkB: scalar, optional (default: 2)
+
+ mutation_scale: scalar, optional (default: 1)
+ Value with which attributes of arrowstyle (e.g., head_length)
@anntzer

anntzer Mar 20, 2017

Contributor

Add some more * here and below?

@NelleV

NelleV Mar 20, 2017

Contributor

noooooooooooooooooo :(

lib/matplotlib/patches.py
@@ -4075,28 +4106,40 @@ def __init__(self, posA=None, posB=None,
self._mutation_aspect = mutation_aspect
self.set_dpi_cor(dpi_cor)
- #self._draw_in_display_coordinate = True
+ # self._draw_in_display_coordinate = True
@anntzer

anntzer Mar 20, 2017

Contributor

are there other mentions of this attribute in the codebase? if it's just a relic of old code it should just be removed.

lib/matplotlib/patches.py
+
+ Parameters
+ ----------
+ connectionstyle: None, ConnectionStyle instance, or string
@anntzer

anntzer Mar 20, 2017

Contributor

None, ConnectionStyle, or str?

@NelleV

NelleV Mar 20, 2017

Contributor

Can you also add a space after the column? It won't render properly elsewise.

@dstansby

dstansby Mar 24, 2017

Contributor

@NelleV I'm confused as to what you mean, my local build seems to look fine.

@NelleV

NelleV Mar 24, 2017

Contributor

It should be connectionstyle : None, ConnectionStyle instance, or string

lib/matplotlib/patches.py
"""
return self._connector
def set_arrowstyle(self, arrowstyle=None, **kw):
"""
- Set the arrow style.
+ Set the arrow style. Old attributes are forgotten. Without arguments
+ (or with `arrowstyle=None`) returns available box styles as a list of
@anntzer

anntzer Mar 20, 2017

Contributor

double backticks

@NelleV

There are spaces missing before columns in the arguments list. Without a space, this will not render properly.

dstansby added some commits Mar 24, 2017

@dstansby dstansby Small fixes
0a660bb
@dstansby dstansby Add spaces before colons
d8286ff
- *mutation_aspect* : The height of the rectangle will be
- squeezed by this value before the mutation and the mutated
- box will be stretched by the inverse of it. default=None.
+ arrow_transmuter :
@QuLogic

QuLogic Mar 26, 2017

Member

This does not appear to be used; maybe it should not be documented? Or marked deprecated or something.

+ connector :
@QuLogic

QuLogic Mar 26, 2017

Member

Also seems to be unused. Actually, both of these are used in Connector, which subclasses from FancyArrowPatch, so I think maybe someone added it here to give it the same API. Maybe just mark these two as ignored.

+ Head and tail patch respectively. :class:`matplotlib.patch.Patch`
+ instance.
+
+ shrinkA, shrinkB : scalar, optional (default: 2)
@QuLogic

QuLogic Mar 26, 2017

Member

"Shrinking factor of tail and head of arrow"?

lib/matplotlib/patches.py
+ ----------
+ connectionstyle : None, ConnectionStyle instance, or string
+ Can be a string with connectionstyle name with
+ optional comma-separated attributes, eg::
@QuLogic

QuLogic Mar 26, 2017

Member

e.g.

lib/matplotlib/patches.py
- set_connectionstyle("arc,angleA=0,armA=30,rad=10")
- set_connectionstyle("arc", angleA=0,armA=30,rad=10)
+ Alternatively, the attributes can be provided as keywords, eg::
@QuLogic

QuLogic Mar 26, 2017

Member

e.g.

lib/matplotlib/patches.py
+ ----------
+ arrowstyle : None, ArrowStyle, str, optional (default: None)
+ Can be a string with arrowstyle name with optional comma-separated
+ attributes, eg.::
@QuLogic

QuLogic Mar 26, 2017

Member

e.g.

lib/matplotlib/patches.py
- Old attrs simply are forgotten.
+ Alternatively attributes can be provided as keywords, eg.::
@QuLogic

QuLogic Mar 26, 2017

Member

e.g.

lib/matplotlib/patches.py
"""
self._mutation_scale = scale
self.stale = True
def get_mutation_scale(self):
"""
Return the mutation scale.
+
+ Returns
+ ----------
@QuLogic

QuLogic Mar 26, 2017

Member

Extra dashes.

QuLogic changed the title from [MRG+1] FancyArrowPatch docstring overhaul to FancyArrowPatch docstring overhaul Mar 26, 2017

@#{review_dismissed_event.actor.login} NelleV dismissed their review Mar 26, 2017

Changes have been applied.

@dstansby dstansby Small fixes
ce10c39

@#{review_dismissed_event.actor.login} anntzer dismissed their review Apr 5, 2017

still waiting for fixing @QuLogic's comments, but mine are fixed.

Contributor

anntzer commented Apr 12, 2017

@dstansby did react to @QuLogic's comments so let's assume until further notice that he plans to address the issues.

Member

QuLogic commented Apr 12, 2017

They've been handled below the lines I commented on; GitHub is not hiding the diff though.

Contributor

anntzer commented Apr 12, 2017

Ah, I see.

@anntzer anntzer merged commit c5c5a5d into matplotlib:master Apr 12, 2017

5 checks passed

codecov/patch 86.66% of diff hit (target 80%)
Details
codecov/project/library 58.41% (+0.45%) compared to 7a152d8
Details
codecov/project/tests 98.58% (target 97.9%)
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

dstansby deleted the dstansby:fancyarrow-doc branch Apr 12, 2017

dstansby moved from Current todo to Done in Arrow drawing overhaul May 25, 2017

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