-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Deprecate parameter orientation of bar() and barh() #17322
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.
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
Conversation
Can't this be done by just using |
It's more a deprecation than a rename. And |
fair enough |
I guess the question, though, remains as to why this should be private. For example we don't have hist() and histh(), just hist(..., orientation=...). It is true that the argument names are not optimal if using the "wrong" orientation, but I'm not sure that's a strong reason for the removal. |
My main motivation is that https://matplotlib.org/devdocs/api/_as_gen/matplotlib.axes.Axes.bar.html should not document If we would design this from scatch, we could use direction-agnostic parameter names. But as is stands, the parameter of I think there should be one official way to draw horizontal bars. Other possibilites would be to deprecate |
I think undocumenting it is good enough. (I'm not blocking the rename, just not so convinced by it.) |
@@ -2392,7 +2392,14 @@ def bar(self, x, height, width=0.8, bottom=None, *, align="center", | |||
error_kw.setdefault('ecolor', ecolor) | |||
error_kw.setdefault('capsize', capsize) | |||
|
|||
orientation = kwargs.pop('orientation', 'vertical') | |||
if 'orientation' in kwargs: | |||
orientation = kwargs['orientation'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we are going to rename we should add a check that we don't get both.
I am 👍 on un-documenting it, 50/50 on renaming it. It is already not in the signature and keyword-only (as we are doing kwargs popping) so the upside of renaming is relatively small (it is already hidden from auto-completers) and we can get it out of the docs by the text describing it from the docstring to a in-line comment. On the other hand, we would make some users change there code to remove some thing that has been documented more-or-less forever (looks like the doc and the functionality came in via f0be66e but was only labeled as "for internal use" in e078f91 for 2.1). |
Can we push this to 3.4? |
I've extracted the un-documentation part to #17504. We can decide for 3.4 if we want to deprecate the parameter in favor of a private version. |
I'm not following up on this. The undocumented state is good enough. |
PR Summary
While stated to be internal, there's a parameter 'orientation' to
bar()
, which is also documented. To make this very clear, this PR deprecates 'orientation' and uses a private '_orientation' parameter instead.The aim is to finally remove 'orientation' also from the docs, because it's rather confusing than helpful to document a parameter and say it's not to be used.