Skip to content

Fix item() reordering dict keys when sort_keys is False#547

Closed
jichaowang02-lang wants to merge 1 commit into
python-poetry:masterfrom
jichaowang02-lang:fix/item-native-dict-key-order
Closed

Fix item() reordering dict keys when sort_keys is False#547
jichaowang02-lang wants to merge 1 commit into
python-poetry:masterfrom
jichaowang02-lang:fix/item-native-dict-key-order

Conversation

@jichaowang02-lang

Copy link
Copy Markdown

Problem

Resolves #546.

In the list-of-dicts branch of item(), the sort-key lambda has a misplaced closing parenthesis:

key=lambda i: (isinstance(i[1], dict), i[0] if _sort_keys else 1)

Only i[0] is guarded by if _sort_keys, so the isinstance(i[1], dict) term stays active even with the default sort_keys=False, forcing dict-valued keys to sort after scalar keys — silently reordering the user's keys. The sibling top-level dict branch already has the correct form (the whole tuple guarded):

key=lambda i: (isinstance(i[1], dict), i[0]) if _sort_keys else 1

The reorder is masked for regular tables (sub-tables are relocated to the end on render anyway) but is directly visible for inline tables, which are not relocated:

>>> from tomlkit.items import item
>>> item([[{"a": {"x": 1}, "b": 2}]]).as_string()
'[[{b = 2, a = {x = 1}}]]'      # before: b emitted before a
'[[{a = {x = 1}, b = 2}]]'      # after:  insertion order preserved

Fix

Move the parenthesis so the whole tuple is guarded by if _sort_keys, matching the sibling branch a few lines above.

Tests

Added test_item_list_of_dicts_preserves_key_order. Full suite passes (pytest tests/ → 1036 passed). CHANGELOG updated.

In the list-of-dicts branch of item(), the sort-key lambda had a
misplaced closing parenthesis:

    key=lambda i: (isinstance(i[1], dict), i[0] if _sort_keys else 1)

Only `i[0]` was guarded by `if _sort_keys`, so the
`isinstance(i[1], dict)` term stayed active even with the default
sort_keys=False, forcing dict-valued keys to sort after scalar keys and
silently reordering the user's keys. The sibling top-level dict branch
already has the correct form (whole tuple guarded):

    key=lambda i: (isinstance(i[1], dict), i[0]) if _sort_keys else 1

The reorder is masked for regular tables (sub-tables are relocated to the
end on render anyway) but is directly visible for inline tables, which
are not relocated. Move the parenthesis to match the sibling branch.

Fixes python-poetry#546.
Copilot AI review requested due to automatic review settings June 29, 2026 07:11

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@dimbleby

dimbleby commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

The point of dictionaries-last is surely to avoid accidental captures - b must come before [a], else the meaning of the document is changed.

If this is unnecessary when not sorting keys, then it also is unnecessary when sorting keys.

On the other hand if it is needed here - then it is needed unconditionally.

@jichaowang02-lang

Copy link
Copy Markdown
Author

You're right, thanks — I had this backwards. The dictionaries-last ordering is doing capture-avoidance: a scalar key has to render before a [sub.table] header, otherwise it gets captured into that table and the document's meaning changes. That's independent of sort_keys, exactly as you say, so making it conditional is wrong — and the failing integration tests confirm it breaks the unsorted path.

The only thing #546 is really about is cosmetic key order inside inline tables (which can't capture), and that's not worth trading capture-safety for. Closing this — sorry for the noise.

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.

item() reorders keys (dict-valued before scalar) even with the default sort_keys=False

3 participants