Skip to content

Matlab Style Label Warns In Test #5244

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

Merged
merged 1 commit into from
Oct 14, 2015

Conversation

rmorshea
Copy link
Contributor

Was receiving warnings on master due to ambiguous matlab style data labeling (use "i" and "j" instead).

@tacaswell
Copy link
Member

@JanSchulz Was that intentional?

@tacaswell tacaswell added this to the next point release (1.5.0) milestone Oct 14, 2015
@jankatins
Copy link
Contributor

I've no idea what this is about :-(

@rmorshea: What are the concrete error messages?

@tacaswell
Copy link
Member

So no 😉

The issue is that the 'y' could be a column or 'yellow' and the test suite is warning us about this Was not sure if you did that as an intentional test or not. This PR just changes the column names to avoid the warning.

tacaswell added a commit that referenced this pull request Oct 14, 2015
@tacaswell tacaswell merged commit 333136b into matplotlib:master Oct 14, 2015
tacaswell added a commit that referenced this pull request Oct 14, 2015
@tacaswell
Copy link
Member

backported as 2d9aa73

@mdboom
Copy link
Member

mdboom commented Oct 14, 2015

Should we open a new issue about this "bug"? (I'd just do it, but I haven't been following the labeling work very closely...) It seems that this ambiguity is a problem. Though it could probably be resolved by assuming 'y' is a label if it's preceded by other labels or is the first argument. That's a horrible heuristic hack, but we are in fact shoehorning two vastly different APIs (matlab-like and pandas-like) together here. There's bound to be rough edges...

@tacaswell
Copy link
Member

There was a long discussion of this in the PR where the labeled data went in. The heuristic is if it is a column, it gets used, but if it could have been a style spec string the user gets a warning.

The winning logic for defaulting to de-referencing the column is that you can get the same style effect, just more verbosely using kwargs.

@rmorshea
Copy link
Contributor Author

It seems pretty weird that one would be warned for using the labels "x" and "y" since it's so common as to be nearly universal. Would it be unreasonable to require that the first two arguments be labels?

@rmorshea rmorshea deleted the matlab_label_warn_in_test branch October 14, 2015 22:49
@mdboom
Copy link
Member

mdboom commented Oct 14, 2015

Requiring the first two arguments to be labels unfortunately would be a major break in behavior. See
http://matplotlib.org/api/pyplot_api.html#matplotlib.pyplot.plot for how flexible it is.

However, I think it is reasonable that if the first argument is a string, then it is a label and so are all other single strings prior to the kwargs. It's an icky heuristic, but I think it might be better than this warning which, as you say, is for a very common case.

@rmorshea
Copy link
Contributor Author

I forgot that plt.plot accepted multiple data sets. Given that, it's not to much a stretch to force stringed args if the first one is, and to me at least, it's better than warning every time since there are plenty of color names that would conflict with standard labels.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants