Skip to content
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
5980e5b
fix working
CloseChoice Sep 28, 2025
f5c2042
WIP: add test
CloseChoice Sep 30, 2025
4f88e03
simplify test case
CloseChoice Sep 30, 2025
21d83a3
speed up checks
CloseChoice Sep 30, 2025
6e35fe9
optimize code
CloseChoice Sep 30, 2025
ae17586
Merge branch 'main' of github.com:pydantic/pydantic-core into fix-122…
CloseChoice Sep 30, 2025
1b30fbc
update test dict
CloseChoice Sep 30, 2025
be8ef2b
remove auto formatting
CloseChoice Sep 30, 2025
bef11c6
update test_dict to reintroduce function I accidentally deleted
CloseChoice Sep 30, 2025
d52d85b
fix strict and updates as request in PR
CloseChoice Oct 1, 2025
0e40c5c
update input python
CloseChoice Oct 1, 2025
c0e6b68
update test_dict using ruff
CloseChoice Oct 1, 2025
a8ffe29
fix linting error on rust
CloseChoice Oct 1, 2025
907ed67
Merge branch 'main' into fix-12273-dict
CloseChoice Oct 1, 2025
9784aa5
Merge branch 'main' of github.com:pydantic/pydantic-core into fix-122…
CloseChoice Oct 1, 2025
e8a8b2a
Merge branch 'fix-12273-dict' of github.com:CloseChoice/pydantic-core…
CloseChoice Oct 1, 2025
7767ad5
try fix performance degradation
CloseChoice Oct 1, 2025
90b27a1
fix performance issue
CloseChoice Oct 2, 2025
6cef2dc
Merge branch 'main' into fix-12273-dict
CloseChoice Oct 2, 2025
84faba3
simplify as discussed in PR
CloseChoice Oct 3, 2025
376a38d
add test
CloseChoice Oct 3, 2025
fd28d80
simplify defaultdict test
CloseChoice Oct 3, 2025
987e3b5
remove tests that were passing before
CloseChoice Oct 3, 2025
939940d
update tests
CloseChoice Oct 3, 2025
7f8e658
update description for graalpy test skipping
CloseChoice Oct 4, 2025
40050eb
Merge branch 'main' into fix-12273-dict
CloseChoice Oct 6, 2025
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
39 changes: 36 additions & 3 deletions src/input/input_python.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,35 @@ use super::{
Input,
};

static ORDERED_DICT_TYPE: PyOnceLock<Py<PyType>> = PyOnceLock::new();

fn get_ordered_dict_type(py: Python<'_>) -> &Bound<'_, PyType> {
ORDERED_DICT_TYPE
.get_or_init(py, || {
py.import("collections")
.and_then(|collections_module| collections_module.getattr("OrderedDict"))
.unwrap()
.extract()
.unwrap()
})
.bind(py)
}

fn check_if_ordered_dict(obj: &Bound<'_, PyAny>) -> bool {
if obj.is_exact_instance_of::<PyDict>() {
return false;
}

if let Ok(type_name) = obj.get_type().name() {
if type_name != "OrderedDict" {
return false;
}
}

let ordered_dict_type = get_ordered_dict_type(obj.py());
obj.is_instance(ordered_dict_type).unwrap_or(false)
}

static FRACTION_TYPE: PyOnceLock<Py<PyType>> = PyOnceLock::new();

pub fn get_fraction_type(py: Python<'_>) -> &Bound<'_, PyType> {
Expand Down Expand Up @@ -395,15 +424,19 @@ impl<'py> Input<'py> for Bound<'py, PyAny> {
Self: 'a;

fn strict_dict<'a>(&'a self) -> ValResult<GenericPyMapping<'a, 'py>> {
if let Ok(dict) = self.downcast::<PyDict>() {
Ok(GenericPyMapping::Dict(dict))
if self.is_exact_instance_of::<PyDict>() {
Ok(GenericPyMapping::Dict(self.downcast::<PyDict>()?))
Copy link
Contributor

Choose a reason for hiding this comment

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

NB downcast != downcast_exact, so this now forbids all subclasses of dict which are not OrderedDict. See my other comment.

} else if check_if_ordered_dict(self) {
Ok(GenericPyMapping::Mapping(self.downcast::<PyMapping>()?))
} else {
Err(ValError::new(ErrorTypeDefaults::DictType, self))
}
}

fn lax_dict<'a>(&'a self) -> ValResult<GenericPyMapping<'a, 'py>> {
if let Ok(dict) = self.downcast::<PyDict>() {
if check_if_ordered_dict(self) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@davidhewitt when I implemented just downcasting to pymapping this resulted in huge performance dips (see here: https://codspeed.io/pydantic/pydantic-core/branches/CloseChoice%3Afix-12273-dict, commit 0e40c5c), therefore I went with this optimzed approach. I can simplify if desired though

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you should do something like

if let Ok(dict) = self.downcast_exact() {
    Ok(GenericPyMapping::Dict(dict))
} else if let Ok(mapping) = self.downcast() { 
    // i.e. treat all subclasses of dict as mappings
    Ok(GenericPyMapping::Mapping))
}

Ok(GenericPyMapping::Mapping(self.downcast::<PyMapping>()?))
} else if let Ok(dict) = self.downcast::<PyDict>() {
Ok(GenericPyMapping::Dict(dict))
} else if let Ok(mapping) = self.downcast::<PyMapping>() {
Ok(GenericPyMapping::Mapping(mapping))
Expand Down
22 changes: 22 additions & 0 deletions tests/validators/test_dict.py
Original file line number Diff line number Diff line change
Expand Up @@ -313,3 +313,25 @@ def test_dict_fail_fast(fail_fast, expected):
v.validate_python({'a': 'b', 'c': 'd'})

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


@pytest.mark.parametrize('strict', [True, False])
def test_ordered_dict_key_order_preservation(strict):
# GH 12273
v = SchemaValidator(cs.dict_schema(keys_schema=cs.str_schema(), values_schema=cs.int_schema()))

# Original issue
foo = OrderedDict({'a': 1, 'b': 2})
foo.move_to_end('a')

result = v.validate_python(foo, strict=strict)
assert list(result.keys()) == list(foo.keys()) == ['b', 'a']
assert result == {'b': 2, 'a': 1}

# More complex case
foo2 = OrderedDict({'x': 1, 'y': 2, 'z': 3})
foo2.move_to_end('x')

result2 = v.validate_python(foo2, strict=strict)
assert list(result2.keys()) == list(foo2.keys()) == ['y', 'z', 'x']
assert result2 == {'y': 2, 'z': 3, 'x': 1}
Loading