Skip to content
This repository was archived by the owner on Jun 3, 2024. It is now read-only.

Updating CI Python images #910

Merged
merged 6 commits into from
Feb 1, 2021
Merged

Updating CI Python images #910

merged 6 commits into from
Feb 1, 2021

Conversation

Marc-Andre-Rivet
Copy link
Contributor

@Marc-Andre-Rivet Marc-Andre-Rivet commented Dec 17, 2020

Companion to plotly/dash#1507

Need to revert back dash branch when merged.

"-3.3.3",
"0..0",
):
for invalid_number in ("10e10000", "e+++eeeeeE-", "12-.3"):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

120.2.33
-3.3.3
0..0

In Chrome 87 the above entries get processed into valid numbers as the invalid characters are simply removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

120.2.33 -> 120.233
-3.3.3 -> -3.33
0..0 -> 0.0

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh interesting - what "removed" means is those characters are not accepted into the input. You type them and they just don't appear.

Begs the question why they still allow you to type 10e10000 - if they know to refuse a second decimal point, why don't they also refuse another zero past 10e100? There's no way to continue typing and turn that into a valid number. Likewise 12-.3 makes no sense at all. Really the only non-numbers they need to allow you to type are incomplete numbers, so I'd probably just put a few of those in and take out these others. Like perhaps -., 12e-. Otherwise we'll just see this problem again when they tighten up their logic further.

@@ -98,7 +98,7 @@ jobs:
. venv/bin/activate && mkdir packages
set -eo pipefail
# build main dash
git clone --depth 1 https://github.com/plotly/dash.git dash-main
git clone -b browser-capabilities --depth 1 https://github.com/plotly/dash.git dash-main
Copy link
Contributor

Choose a reason for hiding this comment

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

@Marc-Andre-Rivet Are we planning to keep this (cloning from a branch) going forward, or just for testing here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just to validate that the changes in plotly/dash#1507 don't break any tests. At least, not in ways we can't explain / fix.

Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

💃

@alexcjohnson alexcjohnson merged commit 6aa850d into dev Feb 1, 2021
@alexcjohnson alexcjohnson deleted the sanity-browser-capabilities branch February 1, 2021 18:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants