Skip to content

Preserve order for collections.OrderedDict #1801

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.

Sign up for GitHub

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

Merged
merged 26 commits into from
Oct 6, 2025
Merged

Preserve order for collections.OrderedDict #1801

Show file tree
Hide file tree
Changes from all 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
6 changes: 4 additions & 2 deletions src/input/input_python.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,15 +395,17 @@ 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>() {
if let Ok(dict) = self.downcast_exact::<PyDict>() {
Ok(GenericPyMapping::Dict(dict))
} else if self.is_instance_of::<PyDict>() {
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 let Ok(dict) = self.downcast_exact::<PyDict>() {
Ok(GenericPyMapping::Dict(dict))
} else if let Ok(mapping) = self.downcast::<PyMapping>() {
Ok(GenericPyMapping::Mapping(mapping))
Expand Down
27 changes: 27 additions & 0 deletions tests/validators/test_dict.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import re
import sys
from collections import OrderedDict
from collections.abc import Mapping
from typing import Any
Expand Down Expand Up @@ -313,3 +314,29 @@ 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.skipif(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note that we skip here due to the bug reported here: #1801 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm interesting, does GraalPy show the bug in a pure Python repro? Otherwise this might imply a PyO3 bug on GraalPy (or a GraalPy C API issue).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

havent managed to recreate this in pure python. So true, this might be a bug in pyo3, but as I understand it, pyo3 is calling the c api mapping code and this is where it goes wrong, therefore I assumed this is ok graalpy's c api, the mapping protocol is not directly exposed in python, so it makes sense that this cannot be reproduced there.

Copy link
Contributor Author

@CloseChoice CloseChoice Oct 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

So I managed to verify that this is a graalpy bug. Here is the code for the C extension:
dict_printer.c:

#include <Python.h>

static PyObject* print_dict(PyObject* self, PyObject* args) {
    PyObject* dict;

    if (!PyArg_ParseTuple(args, "O", &dict)) {
        return NULL;
    }

    PyObject* mapping = PyObject_GetIter(PyMapping_Items(dict));
    if (mapping == NULL) {
        PyErr_SetString(PyExc_TypeError, "Object is not a valid mapping");
        return NULL;
    }

    PyObject* item;
    printf("Mapping contents:\n");
    while ((item = PyIter_Next(mapping)) != NULL) {
        PyObject* key = PyTuple_GetItem(item, 0);
        PyObject* value = PyTuple_GetItem(item, 1);

        PyObject* key_str = PyObject_Str(key);
        PyObject* value_str = PyObject_Str(value);

        const char* key_cstr = PyUnicode_AsUTF8(key_str);
        const char* value_cstr = PyUnicode_AsUTF8(value_str);

        printf("  %s: %s\n", key_cstr, value_cstr);

        Py_DECREF(key_str);
        Py_DECREF(value_str);
        Py_DECREF(item);
    }

    Py_DECREF(mapping);

    if (PyErr_Occurred()) {
        return NULL;
    }

    Py_RETURN_NONE;
}

static PyMethodDef DictPrinterMethods[] = {
    {"print_dict", print_dict, METH_VARARGS, "Print all elements of a dictionary/mapping"},
    {NULL, NULL, 0, NULL}
};

static struct PyModuleDef dictprintermodule = {
    PyModuleDef_HEAD_INIT,
    "dict_printer",
    "A module to print dictionary/mapping elements",
    -1,
    DictPrinterMethods
};

PyMODINIT_FUNC PyInit_dict_printer(void) {
    return PyModule_Create(&dictprintermodule);
}

setup.py

from setuptools import setup, Extension

module = Extension('dict_printer',
                   sources=['dict_printer.c'])

setup(name='dict_printer',
      version='1.0',
      description='C extension to print dictionary/mapping elements',
      ext_modules=[module])

Then run python setup.py build_ext --inplace and execute

from collections import OrderedDict
import dict_printer

my_dict = OrderedDict({'a': 1, 'b': 2, 'c': 3})
my_dict.move_to_end('a')
dict_printer.print_dict(my_dict)
print(my_dict)

This outputs with graalpy:

Mapping contents:
  a: 1
  b: 2
  c: 3
OrderedDict({'b': 2, 'c': 3, 'a': 1})

using CPython:

Mapping contents:
  b: 2
  c: 3
  a: 1
OrderedDict([('b', 2), ('c', 3), ('a', 1)])

So, no worries, no extra work on pyo3 @davidhewitt. I'll check graalpy issues and file one if necessary and link the it in the test-skip string.

EDIT: Here is the graalpy issue I just opened: oracle/graalpython#553

Copy link
Contributor

Choose a reason for hiding this comment

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

Super, thank you for going to such detail to verify this!

sys.implementation.name == 'graalpy',
reason='GraalPy has a bug where PyMapping.items() does not preserve OrderedDict order. See: https://github.com/oracle/graalpython/issues/553',
)
@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