-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| File | Before (bytes) | After (bytes) | | --------- | -------------: | ------------: | | `.whl` | 16265568 | 9447645 | | `.tar.gz` | 7666222 | 6615305 |
f0fddc2
to
0184bd4
Compare
@alexcjohnson if you have time to look this over we'd be grateful for your feedback |
get_data_validator_instance, | ||
) | ||
|
||
# Target Python version for code formatting with Black. | ||
# Must be one of the values listed in pyproject.toml. | ||
BLACK_TARGET_VERSIONS = "py38 py39 py310 py311 py312" | ||
BLACK_TARGET_VERSIONS = "py39 py310 py311 py312" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
revert this line -- needs to remain the same as what's in pyproject.toml
BLACK_TARGET_VERSIONS = "py39 py310 py311 py312" | |
BLACK_TARGET_VERSIONS = "py38 py39 py310 py311 py312" |
filepath = opath.join(outdir, "validators", "_validators.json") | ||
with open(filepath, "w") as f: | ||
f.write(json.dumps(params, indent=4)) | ||
# f.write(str(params)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔪
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the knife symbol mean you want to remove this code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ha, yes, sorry @bmaranville -- by 🔪 I mean delete this line 😄 (the commented-out one)
DataValidator = ValidatorCache.get_validator("", "data") | ||
FramesValidator = ValidatorCache.get_validator("", "frames") | ||
LayoutValidator = ValidatorCache.get_validator("", "layout") | ||
# from .validators import DataValidator, LayoutValidator, FramesValidator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔪
@bmaranville The file I think the correct way is to add it to the list under |
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.) |
@bmaranville nice work, this is a clever solution! Two small thoughts about it:
true=True
false=False
null=None
v = 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 |
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 the contributing notes and understand the structure of the package. In particular, if my PR modifies code of
plotly.graph_objects
, my modifications concern thecodegen
files 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).