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

alt.datum and alt.value generates incorrect error messages #2913

Closed
joelostblom opened this issue Feb 21, 2023 · 6 comments · Fixed by #3750
Closed

alt.datum and alt.value generates incorrect error messages #2913

joelostblom opened this issue Feb 21, 2023 · 6 comments · Fixed by #3750
Labels

Comments

@joelostblom
Copy link
Contributor

Originally reported in #2883 (comment). When using the datum encoding then the message is incorrect:

alt.Chart().mark_point().encode(x=alt.datum(1, wrong_name=1))
SchemaValidationError: Invalid specification

        altair.vegalite.v5.schema.channels.X, validating 'additionalProperties'

        Additional properties are not allowed ('datum', 'wrong_name' were unexpected)

'datum' should not be included above as an incorrect property to the X channel, instead wrong_name should be mentioned as an incorrect parameter to datum. The way to handle this is probably with special error checking inside the datum function (although maybe it needs to also consider which encoding channel it is used for?).


Also mentioned in #2568 (comment) for alt.value. This can look more confusing now that the error message gives parameter hints:

I found one spec that gives an incorrect hint:

import altair as alt
from vega_datasets import data
cars = data.cars.url

alt.Chart(cars).mark_point().encode(
    x='Acceleration:Q',
    y='Horsepower:Q',
    color=alt.value(1)  # should be eg. alt.value('red')
)
SchemaValidationError: `Color` has no parameter named 'value'

Existing parameter names are:
shorthand      bin         legend   timeUnit   
aggregate      condition   scale    title      
bandPosition   field       sort     type       

See the help for `Color` to read the full description of these parameters

It is allowed to use alt.value() in this context, but in this case it should be a string and not a number.

@joelostblom
Copy link
Contributor Author

@binste This might be an issue to keep in mind when working on #2915 since I believe it is also affected of additionalProperties

@joelostblom
Copy link
Contributor Author

joelostblom commented Sep 21, 2023

This seems to have been fixed.partially The error message for alt.value(1) in the second example is now more helpful: SchemaValidationError: '1' is an invalid value for value. Valid values are of type 'object', 'string', or 'null'.

The error for the second example also makes sense now:

SchemaValidationError: `X` has no parameter named 'wrong_name'

Existing parameter names are:
shorthand      bin      scale   timeUnit   
aggregate      field    sort    title      
axis           impute   stack   type       
bandPosition                               

However, when specifying a property that exists, it still shows an error message which is self-contradictory, alt.Chart().mark_point().encode(x=alt.datum(1, bin=True)):

SchemaValidationError: `X` has no parameter named 'bin'

Existing parameter names are:
shorthand      bin      scale   timeUnit   
aggregate      field    sort    title      
axis           impute   stack   type       
bandPosition     

The error message is similar to what we see here #2563 (comment)

@dangotbanned
Copy link
Member

#2913 (comment)

Minimal Repro

Both have identical behavior and meaning

import altair as alt

>>> alt.Chart().mark_point().encode(x=alt.datum(1, bin=True))
>>> alt.Chart().mark_point().encode(x=alt.XDatum(1, bin=True))

You can get a different error by trying method chaining like

import altair as alt

>>> alt.XDatum(1).bin(True)
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
Cell In[6], line 1
----> 1 alt.XDatum(1).bin(True)

File /altair/altair/utils/schemapi.py:1120, in SchemaBase.__getattr__(self, attr)
   1118 except AttributeError:
   1119     _getattr = super().__getattribute__
-> 1120 return _getattr(attr)

AttributeError: 'XDatum' object has no attribute 'bin'

Note

@joelostblom I feel like this might be pretty tricky to fix, since *(Datum|Value) would need to be inferred by the presence of (datum|value) attributes.

This would be the place to start I guess:

def _get_altair_class_for_error(
self, error: jsonschema.exceptions.ValidationError
) -> type[SchemaBase]:
"""
Try to get the lowest class possible in the chart hierarchy so it can be displayed in the error message.
This should lead to more informative error messages pointing the user closer to the source of the issue.
"""
for prop_name in reversed(error.absolute_path):
# Check if str as e.g. first item can be a 0
if isinstance(prop_name, str):
potential_class_name = prop_name[0].upper() + prop_name[1:]
cls = getattr(vegalite, potential_class_name, None)
if cls is not None:
break
else:
# Did not find a suitable class based on traversing the path so we fall
# back on the class of the top-level object which created
# the SchemaValidationError
cls = self.obj.__class__
return cls

@dangotbanned
Copy link
Member

dangotbanned commented Jan 6, 2025

Edit

Opened #3752 and deleted original comment

@joelostblom
Copy link
Contributor Author

Thanks for looking into this @dangotbanned! The fix in #3750 looks great!

@dangotbanned
Copy link
Member

Thanks for looking into this @dangotbanned! The fix in #3750 looks great!

Happy to help @joelostblom

See #3750 for follow-up work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants