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

Pin selenium to avoid doctest breakage from deprecation #2624

Merged
merged 2 commits into from
Jun 30, 2022

Conversation

joelostblom
Copy link
Contributor

Altair's doctests are currently failing due to a deprecation in Selenium 4.3.0 which is a dependency of altair-saver (as reported in altair-viz/altair_saver#104). Until there is a new release of altair-saver that addresses this (maybe with altair-viz/altair_saver#105), this PR make sure we can run doctests by pinning the version of selenium.

There will likely still be confusion among people who use the altair-saver library directly until it sees a new release since selenium 4.3.0 seems to be the default version installed with altair-saver currently.

@joelostblom joelostblom requested a review from mattijn June 30, 2022 18:12
Copy link
Contributor

@mattijn mattijn left a comment

Choose a reason for hiding this comment

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

If I try locally I need double quotes.

pip install "selenium<4.3.0"

Also documented here: https://docs.python.org/3/installing/index.html#basic-usage:

When using comparator operators such as >, < or some other special character which get interpreted by shell, the package name and the version should be enclosed within double quotes

@joelostblom
Copy link
Contributor Author

Good catch! This is unexpected for me since most Unix shells expand variables within double quotes but not single quotes, and I would expect us to want the string to be passed literally to pip without any possible shell expansion taking place. I wonder if it is an OS-specific issue but as you pointed out, let's follow what the Python docs say. I updated to double quotes and will merge if all the tests pass.

@joelostblom joelostblom merged commit 4da4df1 into vega:master Jun 30, 2022
jairideout added a commit to onecodex/onecodex that referenced this pull request Jul 27, 2022
This should (hopefully) fix the failing HTML & PDF exporter tests in `test_reports.py`. See altair-viz/altair_saver#104 and vega/altair#2624

Closes DEV-7078
jairideout added a commit to onecodex/onecodex that referenced this pull request Jul 27, 2022
This should (hopefully) fix the failing HTML & PDF exporter tests in `test_reports.py`. See altair-viz/altair_saver#104 and vega/altair#2624

Closes DEV-7078
jairideout added a commit to onecodex/onecodex that referenced this pull request Jul 27, 2022
This should (hopefully) fix the failing HTML & PDF exporter tests in `test_reports.py`. See altair-viz/altair_saver#104 and vega/altair#2624

Closes DEV-7078
@jairideout jairideout mentioned this pull request Jul 27, 2022
2 tasks
jairideout added a commit to onecodex/onecodex that referenced this pull request Jul 27, 2022
This should (hopefully) fix the failing HTML & PDF exporter tests in `test_reports.py`. See altair-viz/altair_saver#104 and vega/altair#2624

Closes DEV-7078
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.

2 participants