Skip to content

Matlab Style Label Warns In Test - #5244

Merged
tacaswell merged 1 commit into
matplotlib:masterfrom
rmorshea:matlab_label_warn_in_test
Oct 14, 2015
Merged

tacaswell merged 1 commit into
matplotlib:masterfrom
rmorshea:matlab_label_warn_in_test

Conversation

@rmorshea

Copy link
Copy Markdown
Contributor

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

@tacaswell

Copy link
Copy Markdown
Member

@JanSchulz Was that intentional?

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

Copy link
Copy Markdown
Contributor

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

@rmorshea: What are the concrete error messages?

@tacaswell

Copy link
Copy Markdown
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
Copy Markdown
Member

backported as 2d9aa73

@mdboom

mdboom commented Oct 14, 2015

Copy link
Copy Markdown
Member

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
Copy Markdown
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
Copy Markdown
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

mdboom commented Oct 14, 2015

Copy link
Copy Markdown
Member

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
Copy Markdown
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