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

Choose 'png' image compression in BqplotImageView by default #470

Merged
merged 4 commits into from
Oct 18, 2024

Conversation

bmorris3
Copy link
Contributor

Description

Allow users to take advantage of PNG compression via ImageGL from glue-viz/bqplot-image-gl#109. Bumping pin for bqplot-image-gl.

@maartenbreddels
Copy link
Collaborator

The visual change is ok, so we can put the hash somewhere to indicate this is good, this should be documented at https://pytest-mpl.readthedocs.io/en/stable/hash_mode.html

Do you want to try this @bmorris3, or should we ask @astrofrog ?

@dhomeier dhomeier added enhancement New feature or request bqplot-viewers labels Oct 16, 2024
@dhomeier dhomeier changed the title Choose 'png' image compression by default Choose 'png' image compression in BqplotImageView by default Oct 16, 2024
@dhomeier
Copy link
Contributor

@Carifio24, @astrofrog, are there any tools to run the deploy-reference-images job for updating the reference hashes in glue-jupyter-visual-tests, and does one need special permissions for that?

@bmorris3
Copy link
Contributor Author

I experimented a bunch, and settled on generating updated hashes with:

tox -e py311-test-visual -- --mpl-results-path=results

I've put the resulting hashes in the latest commit, I guess we'll see if that's ok?

@dhomeier
Copy link
Contributor

The scatter2d images apparently have not changed at all.

@astrofrog
Copy link
Member

@dhomeier the CircleCI job updates the reference images

Copy link

codecov bot commented Oct 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.07%. Comparing base (54499a8) to head (46c6f90).
Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #470   +/-   ##
=======================================
  Coverage   86.07%   86.07%           
=======================================
  Files          90       90           
  Lines        5242     5242           
=======================================
  Hits         4512     4512           
  Misses        730      730           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dhomeier
Copy link
Contributor

@dhomeier the CircleCI job updates the reference images

Did you enable that job somehow? I noticed one job that required approval before, but first wanted to fix the checksums, and now it seems to have disappeared again.

@astrofrog
Copy link
Member

@dhomeier - the job to update the reference images only happens on main after a PR has been merged. All that is needed for a PR to pass is for the hashes to be correct.

@bmorris3 bmorris3 requested a review from dhomeier October 18, 2024 14:19
@maartenbreddels maartenbreddels merged commit b7ca61f into glue-viz:main Oct 18, 2024
26 checks passed
Copy link
Contributor

@dhomeier dhomeier left a comment

Choose a reason for hiding this comment

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

Thanks all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bqplot-viewers enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants