Skip to content
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

Color histogram bars #216

Merged
merged 15 commits into from
Nov 2, 2023
Merged

Conversation

jo-mueller
Copy link
Contributor

@jo-mueller jo-mueller commented Oct 4, 2023

@dstansby

Fixes #58

This PR expands the feature histogram widget to color the histogram bars according to the color values in the displayed layer. Currently supported:

  • Points layer
  • Vectors layer

I'm not sure how tracks/shapes layer handle features and color, but this can be added relatively easily here.

Tests

  • Pytest: I rewrote the existing tests a bit to make them more specific to vectors/points/other future supported layer and updated the respective baseline images. However, I didn't get the tests to pass with the call to pytest --mpl - it's probably the same issue as in this PR.
  • Interactive test: I checked in the viewer that all controls behave as expected.

@jo-mueller
Copy link
Contributor Author

pre-commit.ci autofix

@dstansby
Copy link
Member

dstansby commented Oct 8, 2023

I've just fixed some failing tests (due to a new Matplotlib release) at #217, so worth merging or rebasing main.

@jo-mueller
Copy link
Contributor Author

jo-mueller commented Oct 8, 2023

@dstansby

Strange. The figures in the baseline folders were saved from the tests themselves using

deepcopy(fig).savefig('./baseline/name_of_test_function.png')

but this generates an incorrect figures size, as it seems. Is there a correct way to save the generated test figures?

As for the other failing tests, I merged the current head into this branch but am not sure why tests are failing.

@jo-mueller
Copy link
Contributor Author

Hi @dstansby ,

could you give me a hint on how to properly create the figures needed for the test to make the checks pass? Would be much appreciated :)

@dstansby
Copy link
Member

dstansby commented Nov 2, 2023

I think the test figures were in the right place, but just needed updating due to changes that happened between opening the PR and now. I've merged in main, and updated the figures, so lets see if the tests pass now 😄

Copy link
Member

@dstansby dstansby left a comment

Choose a reason for hiding this comment

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

Looking good, thanks for the PR 👍

@dstansby dstansby enabled auto-merge November 2, 2023 15:49
@dstansby dstansby merged commit 113e307 into matplotlib:main Nov 2, 2023
14 checks passed
@jo-mueller jo-mueller deleted the color-histogram-bars branch November 2, 2023 16:21
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.

Make a feature histogram exploration widget
2 participants