Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions python/pydantic_core/core_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -4236,6 +4236,7 @@ def definition_reference_schema(
'model_attributes_type',
'dataclass_type',
'dataclass_exact_type',
'default_factory_not_called',
'none_required',
'greater_than',
'greater_than_equal',
Expand Down
4 changes: 4 additions & 0 deletions src/errors/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,9 @@ error_types! {
class_name: {ctx_type: String, ctx_fn: field_from_context},
},
// ---------------------
// Default factory not called (happens when there's already an error and the factory takes data)
DefaultFactoryNotCalled {},
// ---------------------
// None errors
NoneRequired {},
// ---------------------
Expand Down Expand Up @@ -493,6 +496,7 @@ impl ErrorType {
Self::ModelAttributesType {..} => "Input should be a valid dictionary or object to extract fields from",
Self::DataclassType {..} => "Input should be a dictionary or an instance of {class_name}",
Self::DataclassExactType {..} => "Input should be an instance of {class_name}",
Self::DefaultFactoryNotCalled {..} => "The default factory uses validated data, but at least one validation error occurred",
Self::NoneRequired {..} => "Input should be None",
Self::GreaterThan {..} => "Input should be greater than {gt}",
Self::GreaterThanEqual {..} => "Input should be greater than or equal to {ge}",
Expand Down
3 changes: 2 additions & 1 deletion src/errors/validation_exception.rs
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,8 @@ impl PyLineError {
};
write!(output, " {message} [type={}", self.error_type.type_string())?;

if !hide_input {
// special case: don't show input for DefaultFactoryNotCalled errors - there is no valid input
if !hide_input && !matches!(self.error_type, ErrorType::DefaultFactoryNotCalled { .. }) {
let input_value = self.input_value.bind(py);
let input_str = safe_repr(input_value);
write!(output, ", input_value=")?;
Expand Down
2 changes: 1 addition & 1 deletion src/validators/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -668,7 +668,7 @@ fn build_validator_inner(
pub struct Extra<'a, 'py> {
/// Validation mode
pub input_type: InputType,
/// This is used as the `data` kwargs to validator functions
/// This is used as the `data` kwargs to validator functions and default factories (if they accept the argument)
pub data: Option<Bound<'py, PyDict>>,
/// whether we're in strict or lax mode
pub strict: Option<bool>,
Expand Down
15 changes: 10 additions & 5 deletions src/validators/model_fields.rs
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same needs to be done for DCs/TDs

Original file line number Diff line number Diff line change
Expand Up @@ -210,13 +210,18 @@ impl Validator for ModelFieldsValidator {
fields_set_vec.push(field.name_py.clone_ref(py));
fields_set_count += 1;
}
Err(ValError::Omit) => continue,
Err(ValError::LineErrors(line_errors)) => {
for err in line_errors {
errors.push(lookup_path.apply_error_loc(err, self.loc_by_alias, &field.name));
Err(e) => {
state.has_field_error = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to track this on the state? Should we adjust the algorithm to just never call .default_value if there's any errors?

(We could go further and rearrange things a bit so that all default values only get filled in after the whole input is processed, there is some overlap with #1620 🤔)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to track this on the state? Should we adjust the algorithm to just never call .default_value if there's any errors?

It's only relevant for default factories taking the data argument, so if you just have a "static" default it shouldn't matter. And because the logic to call the default value is nested under the current field validator, I can't really "influence" the logic from the model field.

This state approach doesn't look right, but I couldn't find a better way to do so 🤔

(We could go further and rearrange things a bit so that all default values only get filled in after the whole input is processed, there is some overlap with #1620 🤔)

Would this allow using other field values independently from the field order?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see. I missed the default factories guarantee field order; this makes a lot of sense.

Because of the existence of PydanticUndefined as a way to ask for defaults, I think my original proposal actually doesn't make sense. 🤔

... but I do worry about when this flag is reset. Currently once you set it it's true for the rest of the validation. This seems problematic in unions, for example.

Copy link
Member Author

@Viicos Viicos Feb 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed this is one of the reasons it doesn't look right; however, for unions I think it should be alright, as there's a boundary in the validation error.

The error we are handling here is the validation error of the field. If this field is defined like:

class Model(BaseModel):
    a: int | str

And a validation error happens on int, the field validator will try validating against str. At this point, we don't know what happened, and we will only get a val error if validation against str failed as well.

Edit: Actually it might be wrong if we have:

class Model1(BaseModel):
    a: int

class Model2(BaseModel): ...

class Model(BaseModel):
    m: Model1 | Model2

if validation fails for Model1.a, it will mutate the state (if it is the same state than when validating Model).

Now the question is: are unions the only concern? If so, we can make sure we somehow reset this state value on union validation, but there might be other concerns apart from unions :/

match e {
ValError::Omit => continue,
ValError::LineErrors(line_errors) => {
for err in line_errors {
errors.push(lookup_path.apply_error_loc(err, self.loc_by_alias, &field.name));
}
}
err => return Err(err),
}
}
Err(err) => return Err(err),
}
continue;
}
Expand Down
5 changes: 5 additions & 0 deletions src/validators/validation_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ pub struct ValidationState<'a, 'py> {
pub fields_set_count: Option<usize>,
// True if `allow_partial=true` and we're validating the last element of a sequence or mapping.
pub allow_partial: PartialMode,
// Whether at least one field had a validation error. This is used in the context of structured types
// (models, dataclasses, etc), where we need to know if a validation error occurred before calling
// a default factory that takes the validated data.
pub has_field_error: bool,
// deliberately make Extra readonly
extra: Extra<'a, 'py>,
}
Expand All @@ -37,6 +41,7 @@ impl<'a, 'py> ValidationState<'a, 'py> {
exactness: None,
fields_set_count: None,
allow_partial,
has_field_error: false,
extra,
}
}
Expand Down
14 changes: 13 additions & 1 deletion src/validators/with_default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use pyo3::PyVisit;
use super::{build_validator, BuildValidator, CombinedValidator, DefinitionsBuilder, ValidationState, Validator};
use crate::build_tools::py_schema_err;
use crate::build_tools::schema_or_config_same;
use crate::errors::{LocItem, ValError, ValResult};
use crate::errors::{ErrorTypeDefaults, LocItem, ValError, ValResult};
use crate::input::Input;
use crate::py_gc::PyGcTraverse;
use crate::tools::SchemaDict;
Expand Down Expand Up @@ -182,6 +182,18 @@ impl Validator for WithDefaultValidator {
outer_loc: Option<impl Into<LocItem>>,
state: &mut ValidationState<'_, 'py>,
) -> ValResult<Option<Py<PyAny>>> {
if matches!(self.default, DefaultType::DefaultFactory(_, true)) && state.has_field_error {
// The default factory might use data from fields that failed to validate, and this results
// in an unhelpul error.
let mut err = ValError::new(
ErrorTypeDefaults::DefaultFactoryNotCalled,
PydanticUndefinedType::new(py).into_bound(py).into_any(),
);
if let Some(outer_loc) = outer_loc {
err = err.with_outer_location(outer_loc);
}
return Err(err);
}
match self.default.default_value(py, state.extra().data.as_ref())? {
Some(stored_dft) => {
let dft: Py<PyAny> = if self.copy_default {
Expand Down
5 changes: 5 additions & 0 deletions tests/test_errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,11 @@ def f(input_value, info):
('model_attributes_type', 'Input should be a valid dictionary or object to extract fields from', None),
('dataclass_exact_type', 'Input should be an instance of Foobar', {'class_name': 'Foobar'}),
('dataclass_type', 'Input should be a dictionary or an instance of Foobar', {'class_name': 'Foobar'}),
(
'default_factory_not_called',
'The default factory uses validated data, but at least one validation error occurred',
None,
),
('missing', 'Field required', None),
('frozen_field', 'Field is frozen', None),
('frozen_instance', 'Instance is frozen', None),
Expand Down
57 changes: 57 additions & 0 deletions tests/validators/test_with_default.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

from pydantic_core import (
ArgsKwargs,
PydanticUndefined,
PydanticUseDefault,
SchemaError,
SchemaValidator,
Expand Down Expand Up @@ -819,3 +820,59 @@ def _raise(ex: Exception) -> None:
v.validate_python(input_value)

assert exc_info.value.errors(include_url=False, include_context=False) == expected


def test_default_factory_not_called_if_existing_error(pydantic_version) -> None:
class Test:
def __init__(self, a: int, b: int):
self.a = a
self.b = b

schema = core_schema.model_schema(
cls=Test,
schema=core_schema.model_fields_schema(
computed_fields=[],
fields={
'a': core_schema.model_field(
schema=core_schema.int_schema(),
),
'b': core_schema.model_field(
schema=core_schema.with_default_schema(
schema=core_schema.int_schema(),
default_factory=lambda data: data['a'],
default_factory_takes_data=True,
),
),
},
),
)

v = SchemaValidator(schema)
with pytest.raises(ValidationError) as e:
v.validate_python({'a': 'not_an_int'})

assert e.value.errors(include_url=False) == [
{
'type': 'int_parsing',
'loc': ('a',),
'msg': 'Input should be a valid integer, unable to parse string as an integer',
'input': 'not_an_int',
},
{
'input': PydanticUndefined,
'loc': ('b',),
'msg': 'The default factory uses validated data, but at least one validation error occurred',
'type': 'default_factory_not_called',
},
]

assert (
str(e.value)
== f"""2 validation errors for Test
a
Input should be a valid integer, unable to parse string as an integer [type=int_parsing, input_value='not_an_int', input_type=str]
For further information visit https://errors.pydantic.dev/{pydantic_version}/v/int_parsing
b
The default factory uses validated data, but at least one validation error occurred [type=default_factory_not_called]
For further information visit https://errors.pydantic.dev/{pydantic_version}/v/default_factory_not_called"""
)
Loading