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

Deduplicate error messages #2975

Merged
merged 4 commits into from
Mar 18, 2023
Merged

Conversation

binste
Copy link
Contributor

@binste binste commented Mar 18, 2023

Fixes #2896 and this comment:

alt.Chart().encode(alt.Angle().sort("invalid_value")) gives:

'invalid_value' is not of type 'array'
'invalid_value' is not of type 'array'
'invalid_value' is not of type 'array'
'invalid_value' is not of type 'array'

Error messages should be deduplicated based on .message attribute

Further improvements to messages generated out of multiple errors can be tackled separately as part of #2873

@mattijn
Copy link
Contributor

mattijn commented Mar 18, 2023

Thanks @binste!

After this PR, the following examples return as such:

import altair as alt
from vega_datasets import data

alt.Chart().encode(alt.Angle().sort("invalid_value"))
SchemaValidationError: 'invalid_value' is an invalid value for `sort`:

'invalid_value' is not of type 'array'

alt.Chart(...)
source = data.cars()
alt.Chart(source).mark_text(align="right").encode(
    alt.Text("Horsepower:N", bandPosition='4')
)
SchemaValidationError: '4' is an invalid value for `bandPosition`:

'4' is not of type 'number'
Additional properties are not allowed ('field' was unexpected)
Additional properties are not allowed ('bandPosition', 'field', 'type' were unexpected)

alt.Chart(...)
alt.Text("Horsepower:N", type=dict(text="Horsepower", align="right")).to_dict()
SchemaValidationError: '{'text': 'Horsepower', 'align': 'right'}' is an invalid value for `type`:

{'text': 'Horsepower', 'align': 'right'} is not one of ['quantitative', 'ordinal', 'temporal', 'nominal']
{'text': 'Horsepower', 'align': 'right'} is not of type 'string'

No duplication of error messages anymore. Will merge once tests pass.

One question, why did you had to add the $ sign?

@mattijn mattijn merged commit 08737c8 into vega:master Mar 18, 2023
@binste
Copy link
Contributor Author

binste commented Mar 18, 2023

Great, thanks for double-checking! Regarding the $, the expected messages we use in the tests are treated as regex by pytest. $ matches the end of the line, else this test:

SchemaValidationError: 'invalid_value' is an invalid value for `sort`:

'invalid_value' is not of type 'array'

would also pass if the message looks like this:

SchemaValidationError: 'invalid_value' is an invalid value for `sort`:

'invalid_value' is not of type 'array'
'invalid_value' is not of type 'array'
'invalid_value' is not of type 'array'
'invalid_value' is not of type 'array'

I then also added it to the existing examples as an additional restriction just to be safe.

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.

Multiple rounds of the same error message
2 participants