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

Fix __repr__, move it to the base component #492

Merged
merged 6 commits into from
Feb 11, 2019
Merged

Conversation

rmarren1
Copy link
Contributor

This does two things at once:

  1. Fixes some issues in __repr__
  2. (proposed) Move __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)

@T4rk1n
Copy link
Contributor

T4rk1n commented Dec 14, 2018

__repr__ should be defined in the base class.

@rmarren1
Copy link
Contributor Author

rmarren1 commented Dec 14, 2018

Okay, then I'll add something along the lines of

if not _dash.version.__version__ > <version_with_repr>:
    print("Please update the `dash` module to >= <version_with_repr> to use this "
          "version of dash_html_components.\n"
          "You are using version {:s}".format(_dash.version.__version__),
          file=_sys.stderr)

to the current component libraries

default_argtext=default_argtext,
argtext=argtext,
required_props=required_args
)
Copy link
Contributor

Choose a reason for hiding this comment

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

😸

Copy link
Contributor

@T4rk1n T4rk1n left a comment

Choose a reason for hiding this comment

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

Looks good, 💃

@alexcjohnson
Copy link
Collaborator

@T4rk1n if you still want this PR included, please resolve conflicts and merge it.

# Conflicts:
#	dash/development/_py_components_generation.py
@T4rk1n T4rk1n merged commit 9324643 into master Feb 11, 2019
@T4rk1n T4rk1n deleted the simplify-py-classes branch February 11, 2019 19:20
AnnMarieW pushed a commit to AnnMarieW/dash that referenced this pull request Jan 6, 2022
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.

None yet

3 participants