-
Notifications
You must be signed in to change notification settings - Fork 795
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
Incorrect error message when an unknown param value is used #2744
Comments
This is not unique to
The real error is the first one printed below, but the one that actually gets raised is the last one which is really confusing If we do faceting instead of concating, the real error is raised correctly, but note that Altair is still taking three rounds of looping through the validation and finding error messages for some reason (I am not sure why it doesn't stop when it encounters the first error): |
Making some more incremental progress here, it seems like concatenated specs with data and any spec with a condition are just not working with deep validation in general even when the spec is perfectly valid (not sure if this is a regression or they just never worked). Spec with a condition: alt.Chart().mark_bar().encode(
opacity=alt.condition('', alt.value(1), alt.value(0.2))
).to_dict(validate='deep') # Works without `validate='deep'`
Any concat or layer spec with data or a data url: # Works without `validate='deep'` or with empty data
alt.hconcat(alt.Chart('some_data').mark_point()).to_dict(validate='deep')
|
It seems like for concatenated specs, the issue is that the {'data': {'url': 'some_data_or_url'}, 'mark': 'point'} However, for a layered or concatenated chart, the specification looks slightly different: {'layer': [{'mark': 'point'}], 'data': {'url': 'some_data_or_url'}} This results in the the part that says |
Hi @joelostblom, as an experiment, do you like the error messages better if you comment out this line: (That doesn't explain what's wrong with |
I tried that and I agree that it is better for the scenarios that currently don't work with 'deep'. However, ideally we would make everything work with 'deep' because it does make the messages more informative, e.g. this spec: alt.Chart(data.barley()).mark_bar().encode(
x='variety',
y=alt.Y('sum(yield)', stack='asdf'),
) gives the following with 'deep':
But if we remove it, the error message would just say:
I will look a bit more at this now |
Hi @joelostblom, I played around a little more and my best guess is that the first two examples at the top of this issue are related to two separate issues. This comment is only about the first example. If you define a Chart
and then compute
the result is
I believe the issue is that (for this particular Chart) Altair is wrapping too deep (notice how This particular error can be solved by removing these lines, which wrap the condition: https://github.com/altair-viz/altair/blob/6a3db4d9d8685437953237d9b7a8d8f3c99dfb3e/altair/utils/core.py#L672-L674 I'm optimistic this particular bug can be solved by adding some more logic to the lines I linked above, but I haven't tried yet. I don't think this is related to your concatenated charts example (how could it be, since there is no condition in that example). |
That's great! Thank you for looking into that in more detail Chris! Feel free to play around with that logic if you wanted. I am mainly working to solve the concat/layer issue right now and I think I have figured out that it is due to the top level data not being taking into account when evaluating the individual layers/concats, so I am trying to change the logic in one of the other files so that each layered/concated spec is evaluated in the context of the top level data. |
+1 I'm a newbie and I'm seeing this red herring |
Thanks for sharing your frustrations. The feedback mechanism on providing the correct error back to the user is, maybe surprising, more complex than it seems, luckily there is a great proposal in line that aims to improve this significantly. See PR: #2842. |
I just noted that in a spec that contains a condition, using an unknown parameter value anywhere in the spec will always return an error pointing to the condition even when it has nothing to do with actual error. For example, the following spec:
returns:
If we change to
opacity=alt.value(0.6)
we get the expected output:Note that there is nothing wrong the opacity condition here and removing the incorrect
stack
value leads to zero errors. Looking closer at the error raising sequence, it seems like the stack error is always raised first, but if there is a conditional in the spec there will also be a second error raised. Not sure how raising one error leads to the other or why the sequence doesn't stop at the first raised error.It seems like
{'test': 'datum.yield > 0', 'value': 1}
is evaluating against a schema that only contains thecondition
key, so it should have been{'condition': {'test': 'datum.yield > 0', 'value': 1}}
instead and then{'test': 'datum.yield > 0', 'value': 1}
in the next validation.The text was updated successfully, but these errors were encountered: