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

python: Add instance and kind from ValidationError #650

Closed
jpmckinney opened this issue Dec 22, 2024 · 8 comments · Fixed by #654
Closed

python: Add instance and kind from ValidationError #650

jpmckinney opened this issue Dec 22, 2024 · 8 comments · Fixed by #654

Comments

@jpmckinney
Copy link
Contributor

jpmckinney commented Dec 22, 2024

I see the Rust crate's ValidationError has these fields:

pub struct ValidationError<'a> {
/// Value of the property that failed validation.
pub instance: Cow<'a, Value>,
/// Type of validation error.
pub kind: ValidationErrorKind,
/// Path to the value that failed validation.
pub instance_path: Location,
/// Path to the JSON Schema keyword that failed validation.
pub schema_path: Location,
}

instance and kind would be very useful in the Python package's ValidationError, to be able to construct our own error messages (like how fmt::Display uses the kind) and to add other logic related to the kind.

class ValidationError(ValueError):
message: str
schema_path: list[str | int]
instance_path: list[str | int]

@Stranger6667
Copy link
Owner

That's awesome! For the next minor version, I was thinking about overhauling everything related to errors, including customizing error messages for both, Rust & Python.

I think that exposing these fields is a viable option, however, I'd like to spend some extra time on design to ensure features like internationalization (see #382) are ergonomic enough to use.

@Stranger6667
Copy link
Owner

Added these two attributes, I hope they will help with your use case :) I was also thinking that later there could be extra attributes from the Rust’s enum, but it will require some work

@jpmckinney
Copy link
Contributor Author

Thank you!

Yes, things like AdditionalProperties.unexpected and Required.property would be very useful to retain.

For things like ExclusiveMinimum.limit, I could use schema_path to look up the limit, but for the others mentioned, I’d need to reevaluate the validation keyword to get the values needed to construct my own error messages, etc.

@jpmckinney
Copy link
Contributor Author

I’m wondering whether #[derive(FromPyObject)] on the Rust enum would preserve the struct fields? (Instead of #654 which uses a simple Python-style enum for the kind exposed in Python).

@Stranger6667
Copy link
Owner

I've spent a bit of time on complex enums, but it didn't work straight away because I wanted to keep inner exceptions (e.g. in PropertyNames). I think it is doable but I need to dig deeper, hence I kind of postponed this implementation in favor of a simpler one.

I’m wondering whether #[derive(FromPyObject)] on the Rust enum would preserve the struct fields? (Instead of #654 which uses a simple Python-style enum for the kind exposed in Python).

Assuming you meant IntoPyObject (given the conversion direction we need) I think it could be the way to go. I'll try it out soon

@Stranger6667
Copy link
Owner

This one should work - #656

I am going to add some more details into referencing errors and such, so it can be properly introspected.

@Stranger6667
Copy link
Owner

Seems like PyO3 does not support nested enums needed for ReferencingError & PropertyNames :( I'll investigate more and open an issue in PyO3 if needed. Right now these variants will have limited introspection. But hopefully, it should still be possible to build custom error messages based on that limited "inner" message (likely hacky though)

@jpmckinney
Copy link
Contributor Author

Awesome! 🎉

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