Api bar signature by tacaswell · Pull Request #9110 · matplotlib/matplotlib

tacaswell · 2017-08-27T23:13:36Z

PR Summary

Adjusts the signature of bar and barh

closes #7954
closes #8327
closes #8527
closes #8882

PR Checklist

Has Pytest style unit tests
Code is PEP 8 compliant
Documentation is sphinx and numpydoc compliant
Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

I'll add specific tests later tonight, but the existing test do a pretty good job of covering these changes.

anntzer · 2017-08-28T00:50:37Z

lib/matplotlib/axes/_axes.py

        """
        kwargs = cbook.normalize_kwargs(kwargs, mpatches._patch_alias_map)
+
+        matchers = [


why not just

matchers = [ lambda x, height, width=0.8, bottom=None, **kwargs: (False, x, height, width, bottom, kwargs), lambda left, height, width=0.8, bottom=None, **kwargs: (False, left, height, width, bottom, kwargs), ]

?

or did I miss a case?

ah, that is better 🐑

anntzer · 2017-08-28T00:52:02Z

lib/matplotlib/axes/_axes.py

        Make a bar plot with rectangles bounded by:

-          `left`, `left` + `width`, `bottom`, `bottom` + `height`
+          `x` - width/2, `x` + `width`/2, `bottom`, `bottom` + `height`


These should be changed to use double backquotes around each complete expression.

anntzer · 2017-08-28T00:52:38Z

lib/matplotlib/axes/_axes.py

                error_kw["label"] = '_nolegend_'

-            errorbar = self.errorbar(x, y,
+            errorbar = self.errorbar(ex, ey,


just above (line 2183) can be switched to use setdefault.

anntzer · 2017-08-28T00:54:01Z

doc/api/api_changes/2017-08-27-TAC.rst

+  barh(bottom, width, *, align='center', **kwargs)
+
+despite behaving as the center in both cases. The methods now take ``*args, **kwargs``
+is input and are documented to have the primary signatures of ::


no space before ::

anntzer · 2017-08-28T00:59:03Z

lib/matplotlib/axes/_axes.py

+            else:
+                break
+        else:
+            raise TypeError("Missing two required positional arguments: 'x'"


Can you save the first exception and just reraise it here? this will make the message correct in more cases (e.g. the user passes height as a kwarg but not x).

The first one won't be right, it is the 4th one we want, but if you put the 4th one first it will hit and then we would have to handle the kwargs for the two optional ones seperatly (which might be better).

tacaswell · 2017-08-28T01:24:42Z

I think I addressed all of @anntzer 's comments, tests are in-progress.

anntzer · 2017-08-28T01:53:31Z

lib/matplotlib/axes/_axes.py

                                     "ecolor"],
-                      label_namer=None)
+                      label_namer=None,
+                      positional_parameter_names=['x',


Why do we need this? (I'ml not too familiar with the exact semantics of _preprocess_data, but at least y looks a bit fishy in the argument list here)

the 'y' is junk from a second rename that I ended up walking back. The purpose of this list is to tell the pre-processor the name of input hidden in the '*args' so it know which entries to try and look up in data, however in this case we want to do all of them so there is a simpler way.

anntzer · 2017-08-28T01:56:08Z

lib/matplotlib/axes/_axes.py

-            # size width and bottom according to length of left
-            if _bottom is None:
+            # size width and y according to length of x
+            if _y is None:


Can we just default bottom to 0 instead of None? (looks so, except possibly for log scale?)

we would have to keep this logic anyway as users can be passing None in explicitly. Inclined to let that stay as-is for now.

Can we at least make the change go before the call to np.broadcast_arrays, so that you don't have to call zeros_like below?
(I would also deprecate allowing passing None yada yada)

Change the documented first position argument to x and y for bar and barh respectively but still support passing in left and bottom as keyword arguments. closes matplotlib#7954 closes matplotlib#8327 closes matplotlib#8527 closes matplotlib#8882

This change is from matplotlib#8993

anntzer · 2017-08-28T02:49:27Z

CI failing with some docstring markup.

dopplershift

A few minor things, but otherwise it's a 👍 from me.

dopplershift · 2017-08-28T19:17:46Z

lib/matplotlib/axes/_axes.py

-
-        patches = self.bar(left=left, height=height, width=width,
-                           bottom=bottom, orientation='horizontal', **kwargs)
+        matchers = [


A comment explaining what the lambdas are for wouldn't hurt.

@anntzer

credit goes to @anntzer

tacaswell added this to the 2.1 (next point release) milestone Aug 27, 2017

tacaswell added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Aug 27, 2017

anntzer reviewed Aug 28, 2017

View reviewed changes

tacaswell added 3 commits August 27, 2017 22:16

API: change signature of bar and barh

e078f91

Change the documented first position argument to x and y for bar and barh respectively but still support passing in left and bottom as keyword arguments. closes matplotlib#7954 closes matplotlib#8327 closes matplotlib#8527 closes matplotlib#8882

MNT: update pyplot.hist for changes to Axes.hist

5546eee

This change is from matplotlib#8993

STY: whitespace fixes

5bb1c8a

tacaswell force-pushed the api_bar_signature branch from 1ee0351 to 5bb1c8a Compare August 28, 2017 02:16

tacaswell added 2 commits August 27, 2017 23:17

DOC: tweak markup a bit

89c6a77

FIX: bug in docs inserted by _preprocess_data

dd9c7b1

dopplershift requested changes Aug 28, 2017

View reviewed changes

tacaswell added 2 commits August 28, 2017 15:20

DOC: add comment about use of lambdas to unpack args/kwrags

80355e0

credit goes to @anntzer

MNT: re-organize to simplify handling None as input

7e1a65d

dopplershift approved these changes Aug 28, 2017

View reviewed changes

efiring merged commit 6c69f3e into matplotlib:master Aug 31, 2017

tacaswell deleted the api_bar_signature branch August 31, 2017 02:14

anntzer mentioned this pull request Sep 10, 2017

A #9178

Closed

anntzer mentioned this pull request Mar 9, 2018

plt.bar() fails on 2D arrays with singleton dimensions. error message is unhelpful. #10733

Closed

Uh oh!

Conversation

tacaswell commented Aug 27, 2017 • edited by dopplershift Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Checklist

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!

tacaswell commented Aug 28, 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 commented Aug 28, 2017

Uh oh!

dopplershift left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tacaswell commented Aug 27, 2017 •

edited by dopplershift

Loading