Repository navigation
Write emptied AoT fallback under its declaring header - #693
ryanchou1994 wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
Ekko82
left a comment
There was a problem hiding this comment.
Checked 0c0b3867 on Windows with Python 3.13.9. All 98 tests in tests/test_toml_document.py pass. The six added regression cases fail against base d7def7d and pass on this head; the five existing emptied-AoT cases pass on both.
Twelve additional parse/pop/dump cases also pass, covering equivalent quoted keys, deeper declaring headers, multiple emptied siblings, nested array elements, and controls without a declaring header or with a nonempty AoT. Both tomllib and tomlkit recover the mutated document's data, and rendering is unchanged after a second call or a deepcopy. Nine of these cases produce invalid TOML on the base.
|
One header-order case still produces invalid TOML on import tomlkit
import tomllib
source = "[a.b]\nz=1\n[a]\ny=1\n[[a.b.c]]\nx=1\n"
tomllib.loads(source) # valid input
doc = tomlkit.parse(source)
doc["a"]["b"]["c"].pop()
rendered = doc.as_string()
print(rendered)
tomllib.loads(rendered)The output is: [a.b]
z=1
[a]
b.c = []
y=1
This is an uncovered case, not a regression introduced by the PR: it also fails on base |
|
Thanks for the repro. I can reproduce it on 0c0b386 and on the base too, so it's the same fallback with the declaring [a.b] header sitting in an earlier fragment of a. I'll extend the lookup to cover that, add this case and the deeper variant to the tests, and push here. |
|
Pushed 4d0be60. The scope that holds all the fragments of [a.b]
c = []
z = 1
[a]
y = 1Added your case, the deeper |
Ekko82
left a comment
There was a problem hiding this comment.
Rechecked 4d0be60 on Windows with Python 3.13.9. The reported earlier-[a.b] case now emits c = [] under that header, and the deeper variants round-trip correctly through tomllib and tomlkit.
All 102 tests in tests/test_toml_document.py pass. The four cases added in this update fail against 0c0b3867; the other 11 emptied-AoT cases pass there.
Twenty standalone parse/pop/dump probes also pass, including eight earlier-fragment shapes that fail on the old head. These cover quoted keys, multiple emptied siblings, separate outer-AoT elements and retained nonempty siblings. Both parsers recover the mutated data; repeated rendering and deepcopy preserve the output.
An emptied array of tables is rendered as `key = []` at the top of the nearest header scope. When a table on its path is declared by a header in a later fragment (`[[a.b.c]]` ... `[a]`), the hoisted dotted key `a.b.c = []` declared `a` before `[a]`, which is invalid TOML. Pass such fallbacks down to the deepest table whose header declares them, and write them there with a relative key. Fallbacks without a declaring header are rendered as before. Fixes python-poetry#690
When an out-of-order table is split into fragments, the header that declares the parent of an emptied array of tables can sit in an earlier fragment, e.g. [a.b] before [a] and [[a.b.c]]. That header is rendered before the scope the array of tables belongs to, so the fallback was written there as b.c = [], redefining a.b. The scope holding all the fragments now hands such fallbacks to the deepest declaring header before anything is rendered.
4d0be60 to
b359a20
Compare
|
One deeper-header case still emits invalid TOML on import tomlkit
import tomllib
source = "[a.b]\n[[a.b.c.d]]\n[a]\n[a.b.c]\n"
tomllib.loads(source) # valid input
doc = tomlkit.parse(source)
doc["a"]["b"]["c"]["d"].pop()
rendered = doc.as_string()
print(rendered)
tomllib.loads(rendered)The output is: [a.b]
c.d = []
[a]
[a.b.c]Both This also fails on base Checked on Linux / CPython 3.12.15. The current head passes all 1,444 upstream tests, including the toml-test submodule. This was AI-assisted verification, not independent human review. |
The fallback for an emptied array of tables can also need to go to a deeper header in a later fragment, e.g. [a.b] and [[a.b.c.d]] before [a] and [a.b.c]. The fallback was written under [a.b] as c.d = [], which declares a.b.c before the [a.b.c] header does. The fragments were only searched across headers when the repeated key emitted a header itself, so a first fragment that is a super table, or fragments that are all super tables, were missed. Search every fragment of a repeated key, whether or not it emits a header.
|
@manav4u Thanks, that's a real gap. The cross-fragment lookup only ran when the repeated key had a header of its own, so the first fragment of Pushed b131faf, which searches every fragment of a repeated key. Your example now renders as [a.b]
[a]
[a.b.c]
d = []It's in the tests now along with the quoted-key and one-level-deeper variants, one with |
|
✅ Human Attestation: @ryanchou1994 attested this PR at commit
Later pushes to this PR stay attested. Attestation page |
Ekko82
left a comment
There was a problem hiding this comment.
Checked b131faf on Windows with CPython 3.13.9 and pytest 8.4.2. The deeper-header example now places d = [] under [a.b.c]. All six saved pop()/clear() report and control cases preserve the expected data through both tomllib and tomlkit, with stable repeated rendering and deepcopy.
All 163 tests in tests/test_toml_document.py and the 20 previously checked public-API probes pass on this head.
Summary
Since #567, an emptied array of tables is rendered as
key = []at the top of the nearest header scope. If a table on its path is declared by a header in a later fragment ([[a.b.c]]...[a], or[a.b]), the hoisted dotted keya.b.c = []declares that table before its header does, which is invalid TOML (both tomlkit andtomllibreject the output).This change only affects rendering in
tomlkit/container.py: before writing a fallback,_declaring_tablelooks for the deepest table on its path that emits a header in the current scope. If there is one, the fallback is handed to that table and written at the top of its own scope with a relative key, e.g.When the out-of-order table is split into fragments, the scope holding all of them hands the fallback over before anything is rendered, so the declaring header can also be in another fragment (an earlier one, or a deeper header in a later one).
Fallbacks with no declaring header are written exactly as before, and parsing is untouched.
Tests: three parametrized groups (13 shapes, also checked with
tomllibon 3.11+) and three exact-output tests, covering a declaring header in a later fragment, one in an earlier fragment, and a deeper header in another fragment (the last two are the cases reported in the comments below). All 16 fail with master'scontainer.pyand pass with the change. Full suite including the toml-test submodule on Python 3.14 (macOS): 1450 passed.ruff checkandruff format --checkare clean.Fixes #690
Duplicate Check
Agent Drafting Metadata