Skip to content

Fix edgecolor being only applied to first bar. #9353

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 11, 2017

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Oct 10, 2017

closes #9351

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 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 anntzer added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Oct 10, 2017
@anntzer anntzer added this to the 2.1.1 (next bug fix release) milestone Oct 10, 2017
Copy link
Contributor

@afvincent afvincent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine to me. (I guess that ideally one might want to add some test 😈).

@anntzer
Copy link
Contributor Author

anntzer commented Oct 10, 2017

Actually I am thinking to add some machinery to add "as if" comparison tests, which generate two figures and checks that they are indeed identical (in this case, we would be comparing bar([1, 2], ["k"]) and bar([1, 2], ["k", "k"])). This would alleviate the current issue of whether we should try to avoid adding more test reference images which are quite bloating the repo.

@afvincent
Copy link
Contributor

Well that sounds indeed cleverer that the genuine image test I was going to write... Looking quickly at test_axes, it seems like none of the parameters of type 'scalar or iterable' is actually tested for ax.bar: this would be a nice addition to have the kind of test you are thinking about.

@anntzer
Copy link
Contributor Author

anntzer commented Oct 10, 2017

You can already write this "by hand" (https://github.com/matplotlib/matplotlib/pull/9305/files#diff-663c17d0729218d60dbb862c286a17d4R5327), but ideally the machinery should be such that triage_tests.py can display failures correctly.

@tacaswell
Copy link
Member

For this the simplest test is to pass a scalar and then just iterate over the returned artists and check the edge color is what you expect.

@anntzer anntzer force-pushed the bar-edgecolor-cycle branch from fc3e07c to 7ff9287 Compare October 11, 2017 05:19
@anntzer
Copy link
Contributor Author

anntzer commented Oct 11, 2017

tests in

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mpl 2.1 barcharts edgecolor and linewidth only apply to first bar
4 participants