-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
plt.plot fails if given x
and y
kwargs
#4059
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
Comments
@Garrett-R,
|
x
and y
kwargs
@blink1073, thanks I must have realized it at the same time, because I just edited my original post. Please see my new comments. |
@Garrett-R, the problem with your suggestion is that
|
But perhaps the |
With this change your original example works: diff --git a/lib/matplotlib/axes/_axes.py b/lib/matplotlib/axes/_axes.py
index 940ebdd..7db782f 100644
--- a/lib/matplotlib/axes/_axes.py
+++ b/lib/matplotlib/axes/_axes.py
@@ -1365,6 +1365,12 @@ class Axes(_AxesBase):
self.cla()
lines = []
+ if 'y' in kwargs:
+ args = (kwargs.pop('y'),) + args
+
+ if 'x' in kwargs:
+ args = (kwargs.pop('x'),) + args
+
for line in self._get_lines(*args, **kwargs):
self.add_line(line)
lines.append(line) |
I like it! 👍 I wonder if there's any downsides to that. I know the upside. I spent about an hour trying to debug my code, and had to read the the matplotlib source before realizing the |
It could certainly be made more robust, but I agree it is a nice add. I'll let one of the core devs chime in. |
The One concern I have about @blink1073 's patch is that if there is a My knee jerk reaction is 👎 , but am open to being convinced other wise. Also, I don't think any of the docs say that you can pass in the x/y values via kwargs (if there are, please point me to them so they can be removed). |
@tacaswell, that's why I said it needed more clean up. 😄 |
The fact that I could not immediately look at @Garrett-R's example and see why it would fail is a good indicator... |
I share thomas's concern. I have repeatedly made attempts to refactor that If changes are made here, then where else should it go? scatter()? bar()? Nah, I think the real problem here is that there was no error reported. No -1 On Sun, Feb 1, 2015 at 11:15 AM, Steven Silvester notifications@github.com
|
Fair enough. Then, there are two problems: there should be at least one |
It is made extra bad in mpl due to the way that we pass things up (down) the stack so we can not do validation at most levels. I suspect the right place to blow up is in the @WeatherGod I recently dove into the |
Given the reactions from @tacaswell and @WeatherGod, should we close this with no intent to proceed? |
Well, we should probably still raise an error if |
It might even not be too difficult to handle the no-args case with x and/or y kwargs, bypassing all the logic required to handle the Matlab-style parsing. |
This now errors out with "TypeError: plot got an unexpected keyword argument 'x'". |
When I execute the following command:
it outputs:
Edit: I see my error. I was not supposed to provide the data as keyword arguments. Indeed
plt.plot([0,1], [0,1])
works fine.I don't know anything about how matplotlib works, but perhaps
plot
should be able to handle this (similar to howpandas.DataFrame.plot
can handle this):If that's not desirable, then perhaps it could at least throw an error or give a warning that an unexpected keyword argument was given...
The text was updated successfully, but these errors were encountered: