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

Fix png sizing #172

Merged
merged 10 commits into from
Oct 16, 2024
Merged

Fix png sizing #172

merged 10 commits into from
Oct 16, 2024

Conversation

jhyching
Copy link
Collaborator

What this PR does / why we need it:
Fix bad cropping of png and add a bit more functionality.

Which issue(s) this PR fixes
Fixes bad cropping in png introduced by changes to chrome webdriver.

Special notes for your reviewer:

Release note:

Switch custom cropping function to the one provided by Bokeh, as well as enabling some of  the extra functionality.

jhyching and others added 7 commits October 2, 2024 14:00
Changes in either Selenium or the ChromeDriver causes the lower parts of charts to be cropped in the png output. The Bokeh `get_screenshot_as_png` has some smarter calculations to overcome this. Plus it also allows a scale factor to improve output resolution.
tox.ini Outdated Show resolved Hide resolved
@tjofas
Copy link
Collaborator

tjofas commented Oct 15, 2024

Removed Python3.8 and added 3.11 in some different places, prepped for a release 5.0.0 (as we are dropping support for 3.8) and added support to use make black for code formatting.

Is it possible for you @jhyching to change the target of this PR to the branch Release-5.0.0? Otherwise let's open a new PR targeting this branch. That way we adhere to the workflow from Pelle to first merge into a release branch and then do the release when merging that branch to master

@jhyching jhyching changed the base branch from master to Release-5.0.0 October 15, 2024 19:34
Copy link
Collaborator Author

@jhyching jhyching left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@tjofas tjofas merged commit 4fcf369 into spotify:Release-5.0.0 Oct 16, 2024
6 checks passed
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.

3 participants