Repository navigation
Python: merge namespace files instead of replacing them (#403) - #415
Conversation
Generating into an output directory that already holds generated Python code replaced each package's __init__.py with only the current run's types, so types written by earlier runs could no longer be imported from their package. Namespace files are now append-only for languages that set the new merge_namespace_files property (Python only). Each namespace file ends with a manifest comment recording its namespace, its types, and a digest of how they were named. A later run adds its types to those recorded, recomputes the newest-minor-version aliases, and never deletes a file. The manifest is parsed without regard to whitespace or comment characters, so formatters don't affect it. Before writing anything, generation refuses if an existing namespace file has no valid manifest, if this run would name the recorded types differently, if two types' files would differ only in letter case, or if two namespaces would share one namespace file. The new --force flag (force=True in the API) writes the first two kinds with only this run's types; the last two can't be forced. nnvg reports every refusal and exits 1; the API raises NamespaceConflictError. Imports in Python namespace files are now sorted. The verification noxfile generates into a clean directory, and a new slow pytest marker keeps the exhaustive sequence test (G11) to the main CI test job. Docs: a new page on generating into an existing output directory, plus notes in the CLI, language and template guides. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
compat-test-python3-ubuntu failed on Python 3.15: every multiprocessing.Pool after the first test that used tmp_path failed to connect to the forkserver. The noxfile passed --basetemp equal to the nox tmp directory, which nox also exports as TMPDIR. pytest empties a given basetemp when a test first uses tmp_path, and on Python 3.14+ the forkserver (now Linux's default start method) keeps its socket in TMPDIR, so the socket was deleted mid-session. No test on main used tmp_path, so this first showed with the namespace-merge tests. Local runs hid it: their TMPDIR path was too long for a Unix socket, so CPython fell back to /tmp. Class: a pytest session whose --basetemp is the process's TMPDIR. Instances: the test, doctest and rstdoctest sessions in noxfile.py; all three now use a pytest subdirectory. verification/python passes no --basetemp and isn't affected. Sonar: use `bytes | str` for the one Union hint this change adds (S6546), and split the three composite assertions in the new tests (S9073); greps of the diff found no other instances of either. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The maintainer judged that a top-level page elevated a minor consideration to a top-level concept. Remove docs/namespace_merging.rst, its toctree entry, the pointers to it from the CLI and language guides, and the URL in the --force help. The template guide keeps its short section on what a namespace template must do to merge. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|



Fixes #403. Supersedes #410, which was abandoned.
Generating Python into an output directory that already held generated code replaced each package's
__init__.pywith only the current run's types. Types written by earlier runs could then no longer be imported from their package.The change
Namespace files are now append-only for languages that set the new
merge_namespace_filesproperty. Only Python sets it.Before writing anything, generation refuses in four cases (
NamespaceConflictKind), and writes nothing:--forceNO_MANIFESTNAMING_CHANGEDCASE_COLLISIONNAMESPACE_COLLISIONimport/import_), within a run or across runsnnvglists every refused file and exits 1.--dry-run,--list-outputsand--list-inputsreport the same refusals. The API raisesNamespaceConflictError.The guarantees are deliberately weak:
clean → a → b → cwithin one build is the supported workflow.Upgrade impact (release note)
NO_MANIFEST. Clean the directory, or pass--forceonce.namespace_manifest_linesfilter (see the template guide). Otherwise every later run into the same directory refuses.generate_all,generate_all_for_language,generate_all_from_namespaceandgenerate_all_from_namespace_with_generatorsgain a trailingforce=Falseparameter. Existing positional calls are unchanged.main.Gated process artifacts (all approved on #403)
--no-overwrite, manifest placement.NAMESPACE_COLLISION.The design and test plan comments are updated in place for each amendment. G3 and V1 were shown failing on
mainbefore the fix.Self-review and independent reviews
--list-*,--no-overwrite) weren't considered together with the new refusalIndependent review 2 (fresh agent) found N1–N7:
mg.import/mg.import_.NAMESPACE_COLLISIONcan't be forced. Tests U8, G45–G47 and A6.--list-inputsis tested too.get_closest_marker.Independent review 3 (fresh agent) found no code defects, and four low findings, all fixed:
CI and SonarCloud on this PR (fixed in 8a78313):
FileNotFoundErrorconnecting to the forkserver).--basetempwas nox's tmp directory, which nox also exports asTMPDIR. pytest empties a given basetemp the first time a test usestmp_path. On 3.14+, the forkserver (now Linux's default start method) keeps its socket inTMPDIR, so that socket was deleted mid-run.mainusestmp_path./tmp.--basetempisTMPDIR.test,doctestandrstdoctestsessions. All three now use apytestsubdirectory.verification/pythonpasses no--basetemp, so it isn't affected.Unionhint) and S9073 (three composite assertions): every instance in the diff is fixed, and the gate shows 0 new issues.Maintainer review: the standalone docs page was removed (b3040a4) because it made a minor consideration a top-level concept. The template guide keeps a short section on writing a namespace template that merges, and
--force's help describes the refusals.Process fixes, which belong to the maintainer's process draft (not part of this PR):
Defects caught locally, before review:
os.accessreflects permission bits, which isn't true as root.Both are now environment-independent.
Suites run
Run in
ghcr.io/opencyphal/toxic:tx26.4.1(linux/arm64) on the final code:nox -s test-3.11(withNUNAVUT_RUN_SLOW_TESTS=1)doctest-3.11/rstdoctest-3.11test-3.15,test-3.14(checkout on the container's own filesystem, at 8a78313)doctest-3.15/rstdoctest-3.15lint,docsverification/python(nox)blackpassedNot verified locally (CI covers these):
🤖 Generated with Claude Code