Skip to content

Render an emptied array of tables as an empty array - #567

Open
dchaudhari7177 wants to merge 1 commit into
python-poetry:masterfrom
dchaudhari7177:fix/empty-aot-rendering
Open

Render an emptied array of tables as an empty array#567
dchaudhari7177 wants to merge 1 commit into
python-poetry:masterfrom
dchaudhari7177:fix/empty-aot-rendering

Conversation

@dchaudhari7177

Copy link
Copy Markdown

Summary

Addresses the array-of-tables half of #553.

When every element is removed from an array of tables, there is no [[key]] header left to render, and _render_aot looped over an empty body and emitted nothing — so the key vanished from the output entirely:

doc = tomlkit.loads("[[a]]\nx = 1\n")
doc["a"].pop()
tomlkit.dumps(doc)   # '' — the 'a' key is gone

This renders the inline a = [] form instead, which is what tomlkit already produces for an empty array built through the API (doc["a"] = []).

The ordering part

Rendering a = [] in body order is not enough. TOML only reads bare key/value pairs before the first table header, so an emptied array of tables sitting after one would be read back as a key of that table:

[t]
q = 2

a = []      # parses as t.a, not a

So empty arrays of tables are hoisted to just before the first table header. Non-empty ones are untouched and still render in place, byte for byte.

Scope: the other half of the issue

The issue also covers doc["x"]["y"].pop("z") leaving an empty super table that renders as nothing. I have not fixed that here, because the current behaviour is pinned by two existing tests — test_delete_out_of_order_table_key and test_overwriting_out_of_order_table both assert that a super table emptied by del disappears from the output rather than rendering its own header.

Making emptied super tables render [x.y] breaks both. That looked like a deliberate call rather than an oversight, so I left it alone rather than rewriting your tests to suit. Happy to follow up if you'd like that changed — it is a small diff once the intended behaviour is settled.

Tests

Four tests added, covering the bare case, the hoisting case, an emptied array preceded by a scalar, and a control asserting non-empty arrays of tables still round-trip unchanged.

Revert-verified: with container.py reverted, the first three fail and the control still passes.

Full suite on Windows / Python 3.12: 1055 passed (1051 on a clean tree, plus these 4). ruff check and ruff format --check are clean on both changed files.

_render_aot looped over the AoT body to emit [[key]] headers, so an array
of tables with no elements left rendered as nothing at all and the key was
dropped from the output.

Fall back to the inline `key = []` form, matching what an empty array built
through the API already renders as.

TOML only reads bare key/value pairs before the first table header, so these
keys are hoisted there; left in body order an emptied array of tables sitting
after a table would be parsed back as a key of that table. Non-empty arrays
of tables are unaffected and still render in place.

@davidpavlovschi davidpavlovschi 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.

The empty fallback breaks nested arrays of tables because it keeps the full header prefix even though the fallback now renders as a key/value inside the current table scope. Minimal case: parse [t] followed by [[t.a]], pop the only t.a element, then serialize. This head emits [t] / t.a = [], which reparses as t.t.a instead of t.a. The same duplication occurs at deeper tables and inside an AoT element. Please add a nested regression that asserts doc.unwrap() equals parse(doc.as_string()).unwrap(). For an empty AoT rendered inside [t], the key/value fallback needs to be a = [] in that scope, not t.a = []. I ran the full suite on this head as well: 1,054 passed, 1 skipped; the current tests cover only root-level empty AoTs.

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.

2 participants