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

Dash flow tests errors. #319

Closed
T4rk1n opened this issue Aug 2, 2018 · 4 comments
Closed

Dash flow tests errors. #319

T4rk1n opened this issue Aug 2, 2018 · 4 comments

Comments

@T4rk1n
Copy link
Contributor

T4rk1n commented Aug 2, 2018

Running the tests locally, I get this SyntaxError:

  File "C:\Users\t4rk\dev\plotly\dash\tests\test_integration.py", line 8, in <module>
    import dash_flow_example
  File "C:\Users\t4rk\dev\plotly\dash\venv\lib\site-packages\dash_flow_example\__init__.py", line 10, in <module>
    'dash_flow_example'
  File "C:\Users\t4rk\dev\plotly\dash\dash\development\component_loader.py", line 51, in load_components
    namespace
  File "C:\Users\t4rk\dev\plotly\dash\dash\development\base_component.py", line 441, in generate_class
    exec(string, scope)
  File "<string>", line 23
    def __init__(self, id=Component.UNDEFINED, label=Component.REQUIRED, value=Component.UNDEFINED, options=Component.UNDEFINED, config=Component.UNDEFINED, encrypted_*=Component.UNDEFINED, **kwargs):
                                                                                                                                                                        ^
SyntaxError: invalid syntax

The invalid part:
encrypted_*=Component.UNDEFINED, , cannot have a * in a explicit keyword arg (would work in dict passed as kwargs).

When I comment the import dash_flow_example, it works and it somehow works on circleci.

@rmarren1
Copy link
Contributor

rmarren1 commented Aug 7, 2018

encrypted_* is currently not a valid wildcard attribute and would not get parsed out here:

def parse_wildcards(props):
"""
Pull out the wildcard attributes from the Component props
Parameters
----------
props: dict
Dictionary with {propName: propMetadata} structure
Returns
-------
list
List of Dash valid wildcard prefixes
"""
list_of_valid_wildcard_attr_prefixes = []
for wildcard_attr in ["data-*", "aria-*"]:
if wildcard_attr in props.keys():
list_of_valid_wildcard_attr_prefixes.append(wildcard_attr[:-1])
return list_of_valid_wildcard_attr_prefixes

Dash thinks encrypted_* is a normal property and prior to 0.23.1, dash would only complain if you actually tried to set this wildcard attribute here:
https://github.com/plotly/dash-flow-example/blob/81e218e7254ba8c444a53f4c2e477fa8e8af7310/usage.py#L11-L19

Since 0.23.1 dash will try to set encrypted_* as a default (#276), which gives this error.

We can either:

  1. Change encrypted_* to data-* in dash-flow-example
  2. Change encrypted_* to encrypted-* in dash-flow-example and add encrypted-* as a valid wildcard attribute in dash
  3. Remove the notion of valid wildcard attributes in dash, and accept any string of the form '<string>-*' as a wildcard attribute, or even remove the - and accept '<string>*'.

@nicolaskruchten @chriddyp which solution is sufficient for dash-flow-example, I can make a PR for any of the three, just not sure the significance of encrypted_* and how flexible it is.

I'll also see where we can throw a better error message so this issue doesn't come up in the future.

@chriddyp
Copy link
Member

chriddyp commented Aug 7, 2018

Remove the notion of valid wildcard attributes in dash, and accept any string of the form '-' as a wildcard attribute, or even remove the - and accept ''.

This seems reasonable to me. I think that this is how I thought things worked anyway. I don't think that this has any implications besides for Python validation.

I'd like to think about this for a couple of more days just to make sure that there aren't any other long term implications of this.

cc @plotly/dash as well

@rmarren1
Copy link
Contributor

rmarren1 commented Aug 7, 2018

The original use case for wildcards was only to add functionality for the data-* and aria-* attributes for basic HTML elements.

I don't see any technical reason for limiting wildcards, but I also can't really think of a use case where wildcard attributes are necessary.

For example, if you re-implemented a HTML component you could just use data = {attr1: 'value1', ...} and aria = {attr1: 'value1', ...} javascript objects. It seems wildcards only exist in HTML because there was no syntax to represent a map/dict/object, but we have that already in Dash.

@gvwilson
Copy link
Contributor

Hi - we are tidying up stale issues and PRs in Plotly's public repositories so that we can focus on things that are most important to our community. If this issue is still a concern, please add a comment letting us know what recent version of our software you've checked it with so that I can reopen it and add it to our backlog. (Please note that we will give priority to reports that include a short reproducible example.) If you'd like to submit a PR, we'd be happy to prioritize a review, and if it's a request for tech support, please post in our community forum. Thank you - @gvwilson

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

No branches or pull requests

4 participants