FancyArrowPatch docstring overhaul by dstansby · Pull Request #8319 · matplotlib/matplotlib

dstansby · 2017-03-17T18:55:43Z

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.

Docstring overhaul Small fixes

NelleV · 2017-03-17T23:20:39Z

lib/matplotlib/patches.py

-        %(AvailableConnectorstyles)s
+        Parameters
+        ----------
+        posA, posB: None, tuple, optional (default: None)


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

NelleV · 2017-03-17T23:21:20Z

lib/matplotlib/patches.py

+        Parameters
+        ----------
+        posA, posB: None, tuple, optional (default: None)
+            (x,y) co-ordinates of arrow tail and arrow head respectively.


I think it should be "coordinates".

NelleV · 2017-03-17T23:22:27Z

lib/matplotlib/patches.py

+            available:

-        %(AvailableArrowstyles)s
+            %(AvailableArrowstyles)s


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.

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

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).

anntzer · 2017-03-18T20:37:37Z

lib/matplotlib/patches.py

        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)


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

likewise below

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

NelleV

LGTM 👍

anntzer · 2017-03-20T19:34:44Z

lib/matplotlib/patches.py

        """
-        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


... the two points is created.

anntzer · 2017-03-20T19:36:39Z

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)


Add some more * here and below?

noooooooooooooooooo :(

anntzer · 2017-03-20T19:37:07Z

lib/matplotlib/patches.py


        self.set_dpi_cor(dpi_cor)
-        #self._draw_in_display_coordinate = True
+        # self._draw_in_display_coordinate = True


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

anntzer · 2017-03-20T19:38:16Z

lib/matplotlib/patches.py

+
+        Parameters
+        ----------
+        connectionstyle: None, ConnectionStyle instance, or string


None, ConnectionStyle, or str?

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

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

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

anntzer · 2017-03-20T19:38:48Z

lib/matplotlib/patches.py

        """
-        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


double backticks

NelleV

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

QuLogic · 2017-03-26T20:35:11Z

lib/matplotlib/patches.py

-        *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 :


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

QuLogic · 2017-03-26T20:37:10Z

lib/matplotlib/patches.py

+
+            %(AvailableConnectorstyles)s

+        connector :


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.

QuLogic · 2017-03-26T20:38:27Z

lib/matplotlib/patches.py

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


"Shrinking factor of tail and head of arrow"?

QuLogic · 2017-03-26T20:40:39Z

lib/matplotlib/patches.py

+        ----------
+        connectionstyle : None, ConnectionStyle instance, or string
+            Can be a string with connectionstyle name with
+            optional comma-separated attributes, eg::


QuLogic · 2017-03-26T20:40:45Z

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 · 2017-03-26T20:40:57Z

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 · 2017-03-26T20:41:09Z

lib/matplotlib/patches.py

+                set_arrowstyle("Fancy,head_length=0.2")

-        Old attrs simply are forgotten.
+            Alternatively attributes can be provided as keywords, eg.::


QuLogic · 2017-03-26T20:41:29Z

lib/matplotlib/patches.py

        Return the mutation scale.
+
+        Returns
+        ----------


Extra dashes.

Changes have been applied.

@QuLogic

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

anntzer · 2017-04-12T09:18:33Z

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

QuLogic · 2017-04-12T09:21:15Z

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

anntzer · 2017-04-12T09:23:00Z

Ah, I see.

Clean up fancyarrowpatch docstring

1509c86

Docstring overhaul Small fixes

dstansby added the Documentation label Mar 17, 2017

NelleV reviewed Mar 17, 2017

View reviewed changes

anntzer reviewed Mar 18, 2017

View reviewed changes

Small style fixes

42f7bc4

NelleV approved these changes Mar 19, 2017

View reviewed changes

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

anntzer requested changes Mar 20, 2017

View reviewed changes

NelleV previously requested changes Mar 20, 2017

View reviewed changes

dstansby added 2 commits March 24, 2017 20:23

Small fixes

0a660bb

Add spaces before colons

d8286ff

QuLogic reviewed Mar 26, 2017

View reviewed changes

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

Small fixes

ce10c39

anntzer previously approved these changes Apr 5, 2017

View reviewed changes

QuLogic approved these changes Apr 12, 2017

View reviewed changes

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

dstansby deleted the fancyarrow-doc branch April 12, 2017 09:25

QuLogic added this to the v2.1 milestone Aug 12, 2021

Uh oh!

Conversation

dstansby commented Mar 17, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anntzer Mar 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NelleV left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NelleV left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anntzer commented Apr 12, 2017

Uh oh!

QuLogic commented Apr 12, 2017

Uh oh!

anntzer commented Apr 12, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

anntzer Mar 18, 2017 •

edited

Loading