-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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 __repr__, move it to the base component #492
Conversation
c1bc66b
to
343efc5
Compare
|
Okay, then I'll add something along the lines of
to the current component libraries |
default_argtext=default_argtext, | ||
argtext=argtext, | ||
required_props=required_args | ||
) |
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.
Looks good, 💃
@T4rk1n if you still want this PR included, please resolve conflicts and merge it. |
# Conflicts: # dash/development/_py_components_generation.py
This does two things at once:
__repr__
__repr__
to the base component, as there is really no reason it needs to be part of the generated Python classes.I realize now that 2 has the slight issue that a new component using an old version of
dash
will not have its own__repr__
function, and it will not inherit one from the base component, so the resulting__repr__
will be the default.We have required updated component libraries to update
dash
before, e.g. here, but I think these occurrences should be rare, so I'm not really sure if we should also do 2 here and perhaps I should delete it and just do 1.@T4rk1n please review 1, also do you have an opinion on whether we should do 2? (I would also have to re-build core component libraries with and add version checking to core libraries)