More table documentation #12880

timhoffm · 2018-11-24T20:56:04Z

PR Summary

Not all of the table API is great, but at least it's documented now.

This PR does only contain docstring changes, but should not be ported back without #12218.

anntzer · 2018-11-24T21:26:03Z

lib/matplotlib/table.py

@@ -180,7 +180,7 @@ def set_text_props(self, **kwargs):

 class CustomCell(Cell):
    """
-    A subclass of Cell where the sides may be visibly toggled.
+    A subclass of `.Cell` with configurable edge visibility.


A .Cell subclass with...

anntzer · 2018-12-04T23:10:25Z

lib/matplotlib/table.py

@@ -239,13 +246,7 @@ def get_path(self):

 class Table(Artist):
    """
-    Create a table of cells.
-
-    Table can have (optional) row and column headers.


Are these points (this line and the following two) not relevant?

is technically not true for Table. The table is just an arbitrary arrangement of cells. It does not know about headers. The notion of headers is only present in table(), which chooses to optionally specially handle columns and rows with index 0.

Is also not true. Each entry in the table is a Cell (which seems too obvious to mention). Cells are Rectangles with associated Text as written in the cell docstring.

I'm not quite sure about 3) either. Essentially the table is just a 2d array of cells, which don't even have to have the same positions or widths. So the concept of rows and columns is more logical than visual. While there's some size related/position functions like _do_cell_alignment and auto_set_column_width, I would have expected some set_column_width(col, width) and set_row_height(row, height) functions which are clearly not there. Therefore 3) seems an over-statement of the actual capabilities.

anntzer · 2018-12-04T23:12:02Z

lib/matplotlib/table.py

+        ----------------
+        **kwargs
+            `.Artist` properties.
+


Minor preference for no empty line at end of docstring (that makes three lines with effectively no content (empty, closing quotes, empty)).

anntzer · 2018-12-04T23:12:17Z

lib/matplotlib/table.py

+        Parameters
+        ----------
+        ax : `matplotlib.axes.Axes`
+            The `~axes.Axes` to plot the table into.


dot after the tilde?

anntzer · 2018-12-04T23:12:37Z

lib/matplotlib/table.py

-    AXESPAD = 0.02    # the border between the axes and table edge
+
+    AXESPAD = 0.02
+    """The border between the Axes and the table edge."""


in what units?

anntzer · 2018-12-04T23:14:00Z

lib/matplotlib/table.py


        Returns
        -------
-        `CustomCell`: Automatically created cell
+        `CustomCell`


no backquotes needed i think?

anntzer · 2018-12-04T23:15:09Z

lib/matplotlib/table.py

+        This setting does currently only affect newly created cells using
+        `.add_cell`.
+
+        To change existing cells, you have to set their edges explicitly:


Use a code block instead?

blablah:: the code

(if you really want to leave this in doctest format (ugh), note that the second line should have a ... prompt, not >>>)

anntzer · 2018-12-04T23:16:35Z

lib/matplotlib/table.py

-
-        Returns T/F, {}
-        """
+        # docstring inherited


Note that this could additionally return e.g. which cell contained the event (in the dict).
edit: there's already a todo for that.

Currently, the behavior is as documented in the parent. I'm just cleaning up the docs within this PR. So the docstring is redundant and can be left out.

anntzer · 2018-12-04T23:17:15Z

lib/matplotlib/table.py

@@ -475,17 +514,11 @@ def auto_set_column_width(self, col):
        0: the 1st column


the previous line should read "labeling" (not labling)

Docstring rewritten and explanation of indices removed. (Also it's not true: If there are labels, 0 is the label row/column; if not 0 is the first row/column). This is a (IMHO not carefully chosen implementation detail of table() which I do not want to discuss in the docstring of Table.)

anntzer · 2018-12-04T23:17:35Z

lib/matplotlib/table.py

+        As long as auto font size has not been disabled, the value will be
+        clipped such that the text will still fit into the table cell.
+
+        You can disable this behavior Using `.auto_set_font_size`.


using (lowercase)

anntzer · 2018-12-04T23:18:09Z

lib/matplotlib/table.py

+        Notes
+        -----
+        As long as auto font size has not been disabled, the value will be
+        clipped such that the text will still fit into the table cell.


"such that the text fits into the cell" (delete "will still" and "table")
is the fit check only for the horizontal direction? (that's what the end comment suggests) if so, mention it.

anntzer · 2018-12-04T23:19:29Z

lib/matplotlib/table.py

@@ -617,7 +662,18 @@ def _update_positions(self, renderer):
        self._offset(ox, oy)

    def get_celld(self):
-        """Return a dict of cells in the table."""
+        r"""
+        Return a dict of cells in the table mapping indices *(row, column)* to


perhaps delete "indices"

anntzer · 2018-12-04T23:20:25Z

lib/matplotlib/table.py

-          loc='bottom', bbox=None, edges='closed')
+    Add a table to an `~.axes.Axes`.
+
+    You have to specify at least one of *cellText* or *cellColours*. These


I have a preference for passive mode ("At least one of cellText or cellColours must be specified").

anntzer · 2018-12-04T23:21:34Z

lib/matplotlib/table.py

+    cellText : 2D list of str, optional
+        The texts to place into the table cells.
+
+        *Note*: Line breaks in the strings are currently not accounted for and


Could use

.. note:: Line breaks ...

This is rather a comment than something which should stand out. Therefore I intendedly chose not to use an admonition.

timhoffm · 2018-12-05T22:12:10Z

@anntzer Thanks for the thorough review.

anntzer · 2018-12-05T23:45:55Z

lib/matplotlib/table.py


        Returns
        -------
-        `CustomCell`: Automatically created cell
+        cell : `CustomCell`


still no backquotes needed I think?

No, the backquotes are intended to link to CustomCell:

I've added a leading .

anntzer · 2018-12-05T23:46:36Z

lib/matplotlib/table.py

+        ----------
+        col : int or sequence of ints
+            The indices of the columns to auto-scale.
+


remove blank line

anntzer

please self-merge after tests pass

timhoffm · 2018-12-06T19:47:20Z

Won't I need a second review? 😄

jklymak · 2018-12-06T20:33:00Z

Not for doc changes....

timhoffm added the Documentation label Nov 24, 2018

timhoffm added this to the v3.1 milestone Nov 24, 2018

timhoffm mentioned this pull request Nov 24, 2018

plt.table kwarg fontsize does not work #12828

Closed

timhoffm force-pushed the doc-table-2 branch from 5e497ff to f11bb1a Compare November 24, 2018 21:18

anntzer reviewed Nov 24, 2018

View reviewed changes

timhoffm force-pushed the doc-table-2 branch from f11bb1a to 9656a4b Compare November 24, 2018 21:33

timhoffm added the status: needs review label Dec 4, 2018

anntzer reviewed Dec 4, 2018

View reviewed changes

anntzer mentioned this pull request Dec 4, 2018

Shorten an argument check. #12930

Merged

6 tasks

timhoffm force-pushed the doc-table-2 branch 2 times, most recently from 23a9318 to 91f23ca Compare December 5, 2018 22:09

anntzer reviewed Dec 5, 2018

View reviewed changes

More table documentation

131f52b

timhoffm force-pushed the doc-table-2 branch from 91f23ca to 131f52b Compare December 6, 2018 19:32

anntzer approved these changes Dec 6, 2018

View reviewed changes

jklymak merged commit 9c56881 into matplotlib:master Dec 6, 2018

timhoffm deleted the doc-table-2 branch December 6, 2018 22:42

QuLogic removed the status: needs review label Dec 6, 2018

		@@ -475,17 +514,11 @@ def auto_set_column_width(self, col):
		0: the 1st column

Uh oh!

More table documentation #12880

More table documentation #12880

Uh oh!

Conversation

timhoffm commented Nov 24, 2018

PR Summary

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 Dec 4, 2018 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anntzer Dec 4, 2018 • 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!

anntzer Dec 4, 2018 • 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!

timhoffm commented Dec 5, 2018

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 left a comment

Choose a reason for hiding this comment

Uh oh!

timhoffm commented Dec 6, 2018

Uh oh!

jklymak commented Dec 6, 2018

Uh oh!

Uh oh!

anntzer Dec 4, 2018 •

edited

Loading

anntzer Dec 4, 2018 •

edited

Loading

anntzer Dec 4, 2018 •

edited

Loading