Expand mark spec when using to_dict

[joelostblom]

Suggestion to close #2508. To test that the correct chart was created, (e.g. in a student assignment), it would be quite useful it if was possible to test that the functionality of the chart was the same, even if a slightly different syntax was used to create it.

Currently, if I want to test that the correct mark was used in a spec where a mark option might or might not be present, I need to write tests something like this:

chart = alt.Chart().mark_bar(color='blue')

chart_dict = chart.to_dict()
if isinstance(chart_dict['mark'], dict):
    # If the mark takes a param, such as `color='blue'` in this example
    assert chart_dict['mark']['type'] == 'bar'
else:
    # If the spec is just `chart = alt.Chart().mark_bar()`
    assert chart_dict['mark'] == 'bar'

With this PR, I can always write tests like this, which is more convenient:

assert chart.to_dict()['mark']['type'] == 'bar'

This build upon #2813 in spirit, so that to_dict() will always return the "longer version" of a spec, expanding both shorthands to proper VegaLite encodings and with this PR also expanding single mark strings to a dictionary. A notable difference is that the shorthands are altair specific so we have to expand them, whether this expansion of the mark to a dictionary is not technically needed since vega-lite accepts a string too. This means that to_dict() can be used to test the functionality of the chart, whereas chart.mark etc can be used to check if the desired altair syntax was used (e.g. if you want to check that a shorthand was used instead of the long form encoding specification)

[binste]

Only 2 trailing commas in json specs which should be removed, else the implementation looks good to me.

Out of curiosity, is this an issue that is faced with some well-known autograder or a custom built one? If the later, is it worth implementing this in Altair or could the autograder be easily customised?

[joelostblom]

I think this would happen for any test where the input syntax is not controlled, but we want to check the functionality of the generated output spec. For me that has been using the autograder Otter, but I don't see how other autograders would avoid this situation.

While this PR would break a situation where someone has been relying on a test like to find our if a bar mark was used:

assert chart.to_dict()['mark'] == 'bar'

However, I would argue that this test is not ideal because it is checking the syntax used to create the chart rather than the functionality of the generated spec. With this PR, we create two path so that someone who wants to check the syntax used to generate a spec can query the chart object directly and someone who wants to query the functionality of the generated spec can rely on the to_dict method.

To me, this seems worthwhile, but curious to hear what others think.

[mattijn]

Thanks @joelostblom and @binste! The PR is well described and contains clear tests. I support the intention and it is also nice if one want to continue working on the specification in Vega-Lite. No need to expand "mark": "circle" into "mark": {"type": "circle"} anymore to add additional mark properties.