Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.8k
refactor validators to remove duplicate files#5214
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| File | Before (bytes) | After (bytes) || --------- | -------------: | ------------: || `.whl` | 16265568 | 9447645 || `.tar.gz` | 7666222 | 6615305 |
gvwilson commentedJun 4, 2025
@alexcjohnson if you have time to look this over we'd be grateful for your feedback |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
emilykl commentedJun 5, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@bmaranville The file Looks like the correct way is to add |
emilykl commentedJun 5, 2025
Thanks for the PR@bmaranville ! Other than my comments above, looks good to me. The tests are passing and validation seems to be working as usual in the examples I've tried. (@alexcjohnson I'm curious whether you can see any possible pitfalls or edge cases here -- happy to do more testing of specific cases if you have concerns.) |
alexcjohnson commentedJun 6, 2025
@bmaranville nice work, this is a clever solution! Two small thoughts about it:
true=Truefalse=Falsenull=Nonev= to the beginning of the file to convert the JSON to Python, and then @emilykl the only thing I'm not 100% confident of is whether this has any type checking implications. I don't think so, I think the only type checking that matters is on |
gvwilson commentedJun 10, 2025
@alexcjohnson 👍 on reducing whitespace, but I'd like to merge this one first, then combine it with#5218, and then tidy up the whitespace in the result -#5218 switches code formatting and checking from |
Uh oh!
There was an error while loading.Please reload this page.
- fix black exclude parameter in pyproject.toml to properly exclude ONLY directories- reintroduce py38 in codegen/__init__.py- remove commented-out lines- add plotly/validators/_validators.json to pyproject.toml- regenerate code
Uh oh!
There was an error while loading.Please reload this page.
9dc08a1 intomainUh oh!
There was an error while loading.Please reload this page.
emilykl commentedJun 10, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@alexcjohnson I'm in favor of this. I don't think there's any particular reason to keep it human-readable, although it's also not a HUGE difference with compression. Looks like about 194 KB compressed with whitespace vs. 163 KB compressed without whitespace, from a quick test I did locally.
@alexcjohnson@gvwilson I'm in favor of this -- that's a big performance improvement. Something for a future PR? (as a bonus, then we wouldn't have to worry about including this file in the
I'm not aheavy user of type checking so it's possible I'll miss something here. Syntax highlighting is working as expected with these changes. I turned on stricter Pylance type checking and haven't noticed any issues yet but I'll keep an eye out. Hopefully we can put these changes out in an RC to catch any issues before full release. |
This PR replaces#5173 by rebasing@bmaranville's changes on top of
main. Please see#5173 for detailed notes about the motivation and changes. The important changes are in:codegen/init.py
codegen/datatypes.py
codegen/validators.py
doc/python/marker-style.md
plotly/_subplots.py
plotly/basedatatypes.py
plotly/figure_factory/_annotated_heatmap.py
plotly/io/_templates.py
plotly/validator_cache.py
I have read through thecontributing notes and understand the structure of the package. In particular, if my PR modifies code of
plotly.graph_objects, my modifications concern thecodegenfiles and not generated files.I have added tests (if submitting a new feature or correcting a bug) or
modified existing tests.
For a new feature, I have added documentation examples in an existing or
new tutorial notebook (please see the doc checklist as well).
I have added a CHANGELOG entry if fixing/changing/adding anything substantial.
For a new feature or a change in behaviour, I have updated the relevant docstrings in the code to describe the feature or behaviour (please see the doc checklist as well).