NotificationsYou must be signed in to change notification settings
Fork7.9k
Star21.3k

Interpret unitless bar widths for unitized data as relative to spacing.#13254

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Closed

anntzer wants to merge1 commit intomatplotlib:masterfromanntzer:bar-units

Closed

Interpret unitless bar widths for unitized data as relative to spacing.#13254

anntzer wants to merge1 commit intomatplotlib:masterfromanntzer:bar-units

Conversation

Copy link

Contributor

PR Summary

Implements the proposal in#13236.

PR Checklist

Has Pytest style unit tests
Code isFlake 8 compliant
New features are documented, with examples if plot related
Documentation is sphinx and numpydoc compliant
Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

anntzer added the topic: units and array ducktypes label

Jan 21, 2019

anntzer mentioned this pull request

Jan 21, 2019

A reasonable default width for bar() with units?#13236

Open

anntzer force-pushed thebar-units branch froma92d478 to595dd88Compare

January 21, 2019 19:25

Interpret unitless bar widths for unitized data as relative to spacing.

235674f

anntzer force-pushed thebar-units branch from595dd88 to235674fCompare

January 21, 2019 21:01

Copy link

ContributorAuthor

anntzer commentedJan 21, 2019

So this actually breaks some tests that were doing

bar([3, 4, 5], ["foo", "bar", "baz"])

and expected to get

which is pretty nonsensical IMO (with this PR, this fails when trying to add 0 (bottom) to "foo").
Obviously there's backcompat concerns (also for the case of units that's don't define addition -- there's an example in the docs (see the circleci failure), but I'd argue this doesn't really make sense either). Thoughts?