Skip to content

Emit missing-required-key errors in schema-declaration order (#484)#545

Open
uttam12331 wants to merge 1 commit into
alecthomas:masterfrom
uttam12331:fix/required-key-error-order-484
Open

Emit missing-required-key errors in schema-declaration order (#484)#545
uttam12331 wants to merge 1 commit into
alecthomas:masterfrom
uttam12331:fix/required-key-error-order-484

Conversation

@uttam12331

Copy link
Copy Markdown
Contributor

Summary

Closes #484.

Required keys were collected into a set, so the MultipleInvalid raised for missing required keys came out in an arbitrary, hash-seed-dependent order. The reporter's two-key test failed ~1 in 3 runs:

S = Schema({Required('value1'): str, Required('value2'): str, Required('value3'): str})
S({})  # errors sometimes [value1, value2, value3], sometimes [value2, value1, value3], ...

Change

Store the required keys in an insertion-ordered dict (used as an ordered set) instead of a set, and replace the set.discard(...) calls with dict.pop(..., None). The leftover-required-key errors are now emitted in the order the keys were declared in the schema. No other behavior changes.

Verified across several PYTHONHASHSEED values — before, the error order varied per seed; after, it is always declaration order.

Tests

Added test_required_key_error_order_is_stable (5 required keys, asserts the error paths equal the declaration order — an accidental correct ordering on the old code would be ~1 in 120). Full suite passes (170 passed); black --check and flake8 clean. Added a CHANGELOG entry.

@alecthomas

Copy link
Copy Markdown
Owner

This has conflicts that will need to be resolved.

Required keys were collected into a `set`, so the `MultipleInvalid` produced
for missing required keys came out in an arbitrary, hash-seed-dependent order
(the reporter's two-key test failed ~1 in 3 runs).

Store the required keys in an insertion-ordered dict (used as an ordered set)
and replace the `set.discard()` calls with `dict.pop(key, None)`, so the
leftover-required-key errors are emitted in the order the keys were declared
in the schema. Behavior is otherwise unchanged.

Closes alecthomas#484
@uttam12331
uttam12331 force-pushed the fix/required-key-error-order-484 branch from 62c3e2b to 109f955 Compare July 5, 2026 11:45
@uttam12331

Copy link
Copy Markdown
Contributor Author

Thanks @alecthomas — rebased on latest master and resolved the CHANGELOG conflict (moved the #484 entry under the new [Unreleased] section). Full suite still passes (182 passed). Ready for merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MultipleInvalid errors order is not always the same

2 participants