Skip to content

Custom values for griddash, e.g. '10px' don't work (plotly.js 2.12.0) #3710

Closed

Description

@LiamConnors

Custom values for griddash, e.g. '10px' don't work (plotly.js 2.12.0)

Example:

import plotly.express as px
df = px.data.iris()

fig = px.scatter(df, x="sepal_width", y="sepal_length", facet_col="species")
fig.update_xaxes(tickcolor="black", tickwidth=2, ticklen=10, ticklabelmode="period", griddash='10px')
fig.update_yaxes(showgrid=True, gridwidth=1, gridcolor='LightPink')

fig.show()

Gives the following error

ValueError: 
    Invalid value of type 'builtins.str' received for the 'griddash' property of layout.xaxis
        Received value: '10px'

    The 'griddash' property is a string and must be specified as:
      - One of the following strings:
            ['solid', 'dot', 'dash', 'longdash', 'dashdot',
            'longdashdot']
      - A number that will be converted to a string

@archmoj, should it work like this? That throws an error
fig.update_xaxes(showgrid=True, gridwidth=1, gridcolor='LightPink', griddash='10px')

Originally posted by @LiamConnors in #3693 (comment)

Activity

  1. nicolaskruchten commented on May 5, 2022

    @nicolaskruchten
    Contributor

    It looks like our Python-side validation code for 'dash' strings is too restrictive.

  2. archmoj commented on May 9, 2022

    @archmoj
    Contributor

    I suspect this bug exists for every line.dash attribute that has that have "valType": "string" and values e.g.

    "values": [
            "solid",
            "dot",
            "dash",
            "longdash",
            "dashdot",
            "longdashdot"
    ]
  3. archmoj commented on May 9, 2022

    @archmoj
    Contributor

    Here is the current plotly.py validator:

    class DashValidator(EnumeratedValidator):
    """
    Special case validator for handling dash properties that may be specified
    as lists of dash lengths. These are not currently specified in the
    schema.
    "dash": {
    "valType": "string",
    "values": [
    "solid",
    "dot",
    "dash",
    "longdash",
    "dashdot",
    "longdashdot"
    ],
    "dflt": "solid",
    "role": "style",
    "editType": "style",
    "description": "Sets the dash style of lines. Set to a dash type
    string (*solid*, *dot*, *dash*, *longdash*, *dashdot*, or
    *longdashdot*) or a dash length list in px (eg *5px,10px,2px,2px*)."
    },
    """
    def __init__(self, plotly_name, parent_name, values, **kwargs):
    # Add regex to handle dash length lists
    dash_list_regex = r"/^\d+(\.\d+)?(px|%)?((,|\s)\s*\d+(\.\d+)?(px|%)?)*$/"
    values = values + [dash_list_regex]
    # Call EnumeratedValidator superclass
    super(DashValidator, self).__init__(
    plotly_name=plotly_name, parent_name=parent_name, values=values, **kwargs
    )
    def description(self):
    # Separate regular values from regular expressions
    enum_vals = []
    enum_regexs = []
    for v, regex in zip(self.values, self.val_regexs):
    if regex is not None:
    enum_regexs.append(regex.pattern)
    else:
    enum_vals.append(v)
    desc = """\
    The '{name}' property is an enumeration that may be specified as:""".format(
    name=self.plotly_name
    )
    if enum_vals:
    enum_vals_str = "\n".join(
    textwrap.wrap(
    repr(enum_vals),
    initial_indent=" " * 12,
    subsequent_indent=" " * 12,
    break_on_hyphens=False,
    width=80,
    )
    )
    desc = (
    desc
    + """
    - One of the following dash styles:
    {enum_vals_str}""".format(
    enum_vals_str=enum_vals_str
    )
    )
    desc = (
    desc
    + """
    - A string containing a dash length list in pixels or percentages
    (e.g. '5px 10px 2px 2px', '5, 10, 2, 2', '10% 20% 40%', etc.)
    """
    )
    return desc

  4. nicolaskruchten commented on May 9, 2022

    @nicolaskruchten
    Contributor

    Yeah I think that the schema is maybe not quite right in these cases?

    Plotly.py is totally happy with the line.dash attribute in scatter traces though:

    image

  5. nicolaskruchten commented on May 9, 2022

    @nicolaskruchten
    Contributor

    so what's different between scatter.line.dash and the new griddash in the schema?

  6. archmoj commented on May 9, 2022

    @archmoj
    Contributor

    Yeah I think that the schema is maybe not quite right in these cases?

    Plotly.py is totally happy with the line.dash attribute in scatter traces though:

    image

    Could you simply set it to 10px?

  7. nicolaskruchten commented on May 9, 2022

    @nicolaskruchten
    Contributor

    layout.shapes[].line.dash is also problematic.

  8. nicolaskruchten commented on May 9, 2022

    @nicolaskruchten
    Contributor

    Could you simply set it to 10px?

    yes that works also

  9. archmoj commented on May 9, 2022

    @archmoj
    Contributor

    What happens if we do not validate by simply dropping DashValidator function?

  10. archmoj commented on May 9, 2022

    @archmoj
    Contributor

    @alexcjohnson your thoughts?

  11. nicolaskruchten commented on May 9, 2022

    @nicolaskruchten
    Contributor

    Well, the whole purpose of graph_objects is to add validators, so just dropping them seems like the wrong way to go :)

  12. nicolaskruchten commented on May 9, 2022

    @nicolaskruchten
    Contributor

    The problem isn't the DashValidator actually, it's that for attributes like griddash, the validator being used is the StringValidator, because it looks like the codegen isn't able to figure out it needs to use the DashValidator. The DashValidator does admit numbers and the StringValidator does not.

    Here is the code for the scatter.line.dash validator and here is the code for the layout.xaxis.gridddash validator.

  13. nicolaskruchten commented on May 9, 2022

    @nicolaskruchten
    Contributor
  14. nicolaskruchten commented on May 9, 2022

    @nicolaskruchten
    Contributor

    So where else do we need to use the DashValidator ? Looks like layout.shapes[].line.dash and wherever griddash ended up. Anywhere else?

  15. archmoj commented on May 9, 2022

    @archmoj
    Contributor

    Oh, hah, here is why: https://github.com/plotly/plotly.py/blob/master/packages/python/plotly/codegen/utils.py#L184-L201

    Can't we instead say every attribute that ends with dash should be checked by the dash validator?

  16. nicolaskruchten commented on May 9, 2022

    @nicolaskruchten
    Contributor

    It's a bit hard to crawl the whole thing like this I think, which is why we do this enumeration.

  17. nicolaskruchten commented on May 9, 2022

    @nicolaskruchten
    Contributor

    Let's fix this in the next patch release. I'm almost done QA'ing 5.8.0 already.

  18. archmoj commented on May 9, 2022

    @archmoj
    Contributor

    Let's fix this in the next patch release. I'm almost done QA'ing 5.8.0 already.

    Good idea.

  19. alexcjohnson commented on May 9, 2022

    @alexcjohnson
    Collaborator

    The DashValidator does admit numbers and the StringValidator does not.

    FWIW dash attributes are the only ones with valType: 'string' as well as enumerated values:

    Screen Shot 2022-05-09 at 11 57 17

    It's certainly nice the way DashValidator admits both our enumerated strings and the css length list pattern. But it's a pain to need to explicitly list where to use it. Can't we just delegate to it whenever valType: 'string' and a values list appear together?

  20. nicolaskruchten commented on May 9, 2022

    @nicolaskruchten
    Contributor

    I'll see what I can do. @jonmmease originally added this enumeration presumably because he felt another approach might be too brittle in a different direction. string and "has values" seems overly broad and likely to break later, no? Maybe string and "has dash and dotted in values" will be more robust?

  21. alexcjohnson commented on May 9, 2022

    @alexcjohnson
    Collaborator

    Maybe string and "has dash and dotted in values" will be more robust?

    I like that - hard to think what other case we'd want arbitrary strings plus enumerated values in the future, but if there is one it most likely wouldn't include dash and dot :)

  22. nicolaskruchten commented on May 10, 2022

    @nicolaskruchten
    Contributor

    OK so I went with the "ends in dash and is a string" approach instead as it was easier and didn't seem to get anything wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugsomething broken

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions