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

[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)

[nicolaskruchten]

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

[archmoj]

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"
]

[archmoj]

Here is the current plotly.py validator:

plotly.py/packages/python/plotly/_plotly_utils/basevalidators.py

Lines 2245 to 2324 in d5b341c

	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

[nicolaskruchten]

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:

[nicolaskruchten]

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

[archmoj]

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:

Could you simply set it to 10px?

[nicolaskruchten]

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

[nicolaskruchten]

Could you simply set it to 10px?

yes that works also

[archmoj]

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

[archmoj]

@alexcjohnson your thoughts?

[nicolaskruchten]

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

[nicolaskruchten]

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.

[nicolaskruchten]

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

[nicolaskruchten]

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

[archmoj]

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?

[nicolaskruchten]

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

[nicolaskruchten]

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

[archmoj]

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

Good idea.

[alexcjohnson]

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:

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?

[nicolaskruchten]

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?

[alexcjohnson]

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 :)

[nicolaskruchten]

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.