Skip to content

Start replacing ACCEPTS table by parsing numpydoc. #11300

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 26, 2018
Merged

Start replacing ACCEPTS table by parsing numpydoc. #11300

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 15 additions & 8 deletions lib/matplotlib/artist.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1054,17 +1054,16 @@ def mouseover(self, val):

class ArtistInspector(object):
"""
A helper class to inspect an :class:`~matplotlib.artist.Artist`
and return information about it's settable properties and their
current values.
A helper class to inspect an :class:`~matplotlib.artist.Artist` and return
information about its settable properties and their current values.
"""

def __init__(self, o):
"""
Initialize the artist inspector with an
:class:`~matplotlib.artist.Artist` or iterable of :class:`Artists`.
If an iterable is used, we assume it is a homogeneous sequence (all
:class:`Artists` are of the same type) and it is your responsibility
to make sure this is so.
Initialize the artist inspector with an `Artist` or an iterable of
`Artist`\s. If an iterable is used, we assume it is a homogeneous
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This make the tests fail for me: SyntaxError: invalid escape sequence \s Do I have an old version of something lying around?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handled by #11336

sequence (all `Artists` are of the same type) and it is your
responsibility to make sure this is so.
"""
if not isinstance(o, Artist):
if cbook.iterable(o):
Expand Down Expand Up @@ -1135,6 +1134,14 @@ def get_valid_values(self, attr):
match = self._get_valid_values_regex.search(docstring)
if match is not None:
return re.sub("\n *", " ", match.group(1))

# Much faster than list(inspect.signature(func).parameters)[1],
# although barely relevant wrt. matplotlib's total import time.
param_name = func.__code__.co_varnames[1]
match = re.search("(?m)^ *{} : (.+)".format(param_name), docstring)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not quite sure, what we want the exact sematics to be.

What about foo : int, optional, default: None?

Do we want just int or int, optional, default: None?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are all setters, so the first argument "should" never be optional or have a default. As always, there are exceptions (set_xlim(left=...)), but again the point is to handle these with the fallback ACCEPTS table.

I don't think it's worth adding more machinery to this.

if match:
return match.group(1)

return 'unknown'

def _get_setters_and_targets(self):
Expand Down
46 changes: 7 additions & 39 deletions lib/matplotlib/axes/_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -602,8 +602,6 @@ def set_figure(self, fig):
"""
Set the `.Figure` for this `.Axes`.

.. ACCEPTS: `.Figure`

Parameters
----------
fig : `.Figure`
Expand Down Expand Up @@ -907,14 +905,9 @@ def set_axes_locator(self, locator):
"""
Set the axes locator.

.. ACCEPTS: a callable object which takes an axes instance and
renderer and returns a bbox.

Parameters
----------
locator : callable
A locator function, which takes an axes and a renderer and returns
a bbox.
locator : Callable[[Axes, Renderer], Bbox]
"""
self._axes_locator = locator
self.stale = True
Expand Down Expand Up @@ -1022,10 +1015,10 @@ def cla(self):
except TypeError:
pass
# update the minor locator for x and y axis based on rcParams
if (rcParams['xtick.minor.visible']):
if rcParams['xtick.minor.visible']:
self.xaxis.set_minor_locator(mticker.AutoMinorLocator())

if (rcParams['ytick.minor.visible']):
if rcParams['ytick.minor.visible']:
self.yaxis.set_minor_locator(mticker.AutoMinorLocator())

if self._sharex is None:
Expand Down Expand Up @@ -1125,9 +1118,8 @@ def get_facecolor(self):
get_fc = get_facecolor

def set_facecolor(self, color):
"""Set the Axes facecolor.

.. ACCEPTS: color
"""
Set the Axes facecolor.

Parameters
----------
Expand Down Expand Up @@ -1314,8 +1306,6 @@ def set_adjustable(self, adjustable, share=False):
If ``True``, apply the settings to all shared Axes.
Default is ``False``.

.. ACCEPTS: [ 'box' | 'datalim']

See Also
--------
matplotlib.axes.Axes.set_aspect
Expand Down Expand Up @@ -2132,8 +2122,6 @@ def set_autoscale_on(self, b):
"""
Set whether autoscaling is applied on plot commands

.. ACCEPTS: bool

Parameters
----------
b : bool
Expand All @@ -2145,8 +2133,6 @@ def set_autoscalex_on(self, b):
"""
Set whether autoscaling for the x-axis is applied on plot commands

.. ACCEPTS: bool

Parameters
----------
b : bool
Expand All @@ -2157,8 +2143,6 @@ def set_autoscaley_on(self, b):
"""
Set whether autoscaling for the y-axis is applied on plot commands

.. ACCEPTS: bool

Parameters
----------
b : bool
Expand Down Expand Up @@ -2200,8 +2184,6 @@ def set_xmargin(self, m):
I.e. for a data range [0, 2], a factor of ``m = -0.1`` will result in
a range [0.2, 1.8].

.. ACCEPTS: float greater than -0.5

Parameters
----------
m : float greater than -0.5
Expand All @@ -2224,8 +2206,6 @@ def set_ymargin(self, m):
I.e. for a data range [0, 2], a factor of ``m = -0.1`` will result in
a range [0.2, 1.8].

.. ACCEPTS: float greater than -0.5

Parameters
----------
m : float greater than -0.5
Expand Down Expand Up @@ -2306,8 +2286,6 @@ def set_rasterization_zorder(self, z):
z : float or None
zorder below which artists are rasterized. ``None`` means that
artists do not get rasterized based on zorder.

.. ACCEPTS: float or None
"""
self._rasterization_zorder = z
self.stale = True
Expand Down Expand Up @@ -2636,8 +2614,6 @@ def set_frame_on(self, b):
"""
Set whether the axes rectangle patch is drawn.

.. ACCEPTS: bool

Parameters
----------
b : bool
Expand All @@ -2655,8 +2631,6 @@ def set_axisbelow(self, b):
"""
Set whether axis ticks and gridlines are above or below most artists.

.. ACCEPTS: [ bool | 'line' ]

Parameters
----------
b : bool or 'line'
Expand Down Expand Up @@ -3268,11 +3242,9 @@ def set_xticklabels(self, labels, fontdict=None, minor=False, **kwargs):
"""
Set the x-tick labels with list of string labels.

.. ACCEPTS: list of string labels

Parameters
----------
labels : list of str
labels : List[str]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did we somewhere agree to switch to type annotation syntax?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grepping for 'List[' shows that it's definitely already being used.

lib/matplotlib/contour.py
576:        lw = self.get_label_width(self.labelLevelList[lmin],
577:                                  self.labelFmt, self.labelFontSizeList[lmin])
591:        self.add_label(xmin, ymin, rotation, self.labelLevelList[lmin],
592:                       self.labelCValueList[lmin])
1039:        artists : List[`.Artist`]
1042:        labels : List[str]
1810:        hatches : List[str], optional

lib/matplotlib/collections.py
386:        urls : List[str] or None
387:            .. ACCEPTS: List[str] or None

lib/matplotlib/tight_layout.py
52:    num1num2_list : List[int]

lib/matplotlib/widgets.py
515:        labels : List[str]
518:        actives : List[bool], optional
2484:    button : List[Int], optional

lib/matplotlib/backend_bases.py
1257:    callbacks : List[Tuple[callable, Tuple, Dict]]
2368:        callbacks : List[Tuple[callable, Tuple, Dict]]

lib/matplotlib/axes/_base.py
1613:        v : List[float] or one of the strings listed below.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll accept this for now, even tough I would like to have it discussed and agreed upon. I'm not sure if this is really the best way. Whatever the policy is, it needs documentation. I'll leave the topic for #10225.

List of string labels.

fontdict : dict, optional
Expand Down Expand Up @@ -3605,11 +3577,9 @@ def set_yticklabels(self, labels, fontdict=None, minor=False, **kwargs):
"""
Set the y-tick labels with list of strings labels.

.. ACCEPTS: list of string labels

Parameters
----------
labels : list of str
labels : List[str]
list of string labels

fontdict : dict, optional
Expand Down Expand Up @@ -3747,8 +3717,6 @@ def set_navigate(self, b):
"""
Set whether the axes responds to navigation toolbar commands

.. ACCEPTS: bool

Parameters
----------
b : bool
Expand Down
4 changes: 3 additions & 1 deletion lib/matplotlib/tests/test_artist.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,10 +262,12 @@ def test_None_zorder():
("ACCEPTS: [ '-' | '--' | '-.' ]", "[ '-' | '--' | '-.' ] "),
('ACCEPTS: Some description.', 'Some description. '),
('.. ACCEPTS: Some description.', 'Some description. '),
('arg : int', 'int'),
('arg : int\nACCEPTS: Something else.', 'Something else. '),
])
def test_artist_inspector_get_valid_values(accept_clause, expected):
class TestArtist(martist.Artist):
def set_f(self):
def set_f(self, arg):
pass

TestArtist.set_f.__doc__ = """
Expand Down