From 0f2f1cd79774a35f3e7f350002774568d097cebb Mon Sep 17 00:00:00 2001 From: Emre Date: Mon, 14 Sep 2026 14:23:31 +0300 Subject: [PATCH 1/2] Correct three wrong claims in the upstream report record All three were checked against the GitHub API rather than against memory. "Two maintainers approved it." One did: 777arc, 2026-08-17T21:52:30Z, nine minutes after the PR opened. The reviews endpoint returns exactly one review. Teque5 drove the discussion and wrote the PR, which is participation and not a second approval; the line now says so, and says what it used to say. "The accessor shipped as declared_version, not as __original_version. Reading the diff would have given the wrong name." Wrong on both counts. The string __original_version appears nowhere upstream in that spelling -- not in the issue, not in the PR body -- and the PR has named _declared_version since it was opened on 2026-08-17, so reading the diff would have given the right name. The claim is removed and replaced with what actually happened. What actually happened is better than the version being corrected, which is why it is now recorded instead. The maintainer's 2026-08-17 comment ended with three candidate names -- .declared_version, .file_version, .original_version -- and asked which. The reply from here on 2026-08-19 picked declared_version over original_version. It shipped as public declared_version. The shortlist was upstream's, the pick was endorsed from here, and neither side invented it alone. Third: the paragraph introducing the get_global_info() note said the detail "differs from what was predicted", while the note itself says it was predicted correctly from a stand-in and then confirmed. The lead-in contradicted its own content and is rewritten. Everything else in the file was checked and holds: issue filed 2026-08-10, PR opened 2026-08-17 (seven days), the PR body carrying deepcopy, the self.version removal, "Closes #159" and "increment to v1.13.0", and the monthly call being named by the maintainer in-thread. Co-Authored-By: Claude Opus 5 --- docs/sigmf-python-issue-draft.md | 41 ++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/docs/sigmf-python-issue-draft.md b/docs/sigmf-python-issue-draft.md index 93f4592..63f8fb7 100644 --- a/docs/sigmf-python-issue-draft.md +++ b/docs/sigmf-python-issue-draft.md @@ -18,7 +18,11 @@ this file. deep-copies the metadata in `__init__` *and* preserves the declared value. It also removes the errant `self.version` attribute in favour of reading it from the metadata, and closes #159 explicitly. -- Two maintainers approved it, and it shipped in **v1.13.0**. +- One maintainer approved it — `777arc`, 2026-08-17T21:52:30Z, nine minutes + after the pull request was opened — and it shipped in **v1.13.0**. A second + maintainer, `Teque5`, drove the discussion and wrote the PR; that is + participation rather than a second review, and this line previously said + "two maintainers approved" on the strength of it. **Verified on the released version, not on the pull request.** CI resolves dependencies fresh (`uv.lock` is not committed), so 1.13.0 reached CI before it @@ -33,18 +37,29 @@ then measured against an installed 1.13.0: | declared value recoverable from the handle | no | **yes, `handle.declared_version`** | | `handle.get_global_info()["core:version"]` | `1.2.6` | `1.2.6` (unchanged) | -Two details worth recording, because both differ from what the pull request -description suggested: - -1. The accessor shipped as a public **`declared_version`** property (backed by - `_declared_version`), not as `__original_version`. Reading the diff would - have given the wrong name; measuring the release gave the right one. -2. `get_global_info()` **still returns the library's spec version**. The - deepcopy moved the normalisation into the handle's own copy rather than - removing it. This was predicted from a stand-in before 1.13.0 existed and is - now confirmed against the real thing — so `iqforge info` continues to print - `1.0.0 (file); 1.2.6 (reader)`, and that display is correct rather than a - leftover. +One detail worth recording, because it is the half of the fix that did *not* +change what a reader sees: + +`get_global_info()` **still returns the library's spec version**. The deepcopy +moved the normalisation into the handle's own copy rather than removing it, +which is Option B in the maintainer's own framing. This was predicted from a +stand-in before 1.13.0 existed and is now confirmed against the real thing — so +`iqforge info` continues to print `1.0.0 (file); 1.2.6 (reader)`, and that +display is correct rather than a leftover. + +**Where the name came from.** The maintainer's 2026-08-17 comment closed with +three candidates — `.declared_version`, `.file_version`, `.original_version` — +and asked which. The reply from here on 2026-08-19 was that "`declared_version` +reads better to me than `original_version`, but that's a detail". The pull +request had already used `self._declared_version` internally since it was +opened on 2026-08-17, and the released property is public `declared_version`. + +So the shortlist was upstream's and the pick was endorsed from here; neither +side invented it alone. An earlier version of this section claimed the pull +request had called the attribute `__original_version` and that reading the diff +would therefore have given the wrong name. That was wrong on both counts: +`__original_version` appears nowhere upstream in that spelling, and the PR body +named `_declared_version` from the day it was opened. **Checked against the three real captures this report cites.** Their metadata was re-fetched and read under 1.13.0; all three declare `core:version: 1.0.0`, From 42f8c579e442884db7d864640bc39deb33775027 Mon Sep 17 00:00:00 2001 From: Emre Date: Mon, 14 Sep 2026 14:48:07 +0300 Subject: [PATCH 2/2] Stop repeating the __original_version claim in four more places The claim corrected in the previous commit had been copied into four other files. Each said the pull request named the attribute __original_version and that it shipped under a different name. Neither half is true: the PR body has read `add self._declared_version` since it was opened on 2026-08-17, and the string __original_version appears nowhere upstream in that spelling. What actually happened is now written where the wrong version was. The maintainer's issue comment ended with three candidates -- .declared_version, .file_version, .original_version -- and asked which; the reply from here picked the first, and that is what shipped. ROADMAP carried the second error too, "two maintainers approved", and now says one approved nine minutes after the PR opened. tests/test_io.py keeps its conclusion -- membership in FIXED_SIGMF_VERSIONS is earned by measuring an installed release, not by reading a diff -- but it needed a reason that is true. The old one was that the diff would have given the wrong name. The real one is that the half of the fix which matters downstream is invisible in the diff: get_global_info() still returns the library's spec version, and no changelog line said so. Comment only; no behaviour change. The remaining matches for "original_version" in the tree are deliberate: the maintainer's three-name shortlist, quoted, and the self-correction in docs/sigmf-python-issue-draft.md that explains what the old claim said. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 7 ++++--- ROADMAP.md | 7 ++++--- docs/release-notes/v0.5.0.md | 11 ++++++----- tests/test_io.py | 9 +++++---- 4 files changed, 19 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6aac466..cf8bb23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -161,9 +161,10 @@ Nothing yet. by [#160](https://github.com/sigmf/sigmf-python/pull/160)): constructing a `SigMFFile` no longer rewrites the caller's metadata dict. The tripwire in `tests/test_io.py` fired on the release and now records 1.13.0 as verified - non-mutating, measured rather than assumed -- the accessor the fix added is - called `declared_version`, not the `__original_version` the pull request - described. Nothing in `iqforge` changes: `load()` reads `core:version` out of + non-mutating, measured rather than assumed. The accessor the fix added is a + public `declared_version` property; the name came from a three-candidate + shortlist the maintainer put in the issue thread, and was picked from here. + Nothing in `iqforge` changes: `load()` reads `core:version` out of the parsed JSON before handing the dict over, which is correct under both behaviours, so the workaround became redundant rather than wrong. The `sigmf>=1.11.1` floor deliberately stays where it is. `iqforge info` still diff --git a/ROADMAP.md b/ROADMAP.md index 1169397..ff0cc50 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -166,9 +166,10 @@ After Now is done — still reliability-first: ([sigmf-python#159](https://github.com/sigmf/sigmf-python/issues/159)) went from filed to code in **seven days**: it was discussed at SigMF's monthly call, [#160](https://github.com/sigmf/sigmf-python/pull/160) was - opened against it, two maintainers approved, and **both** suggested fixes - were taken rather than just the preferred one — deepcopy in `__init__` - *and* an `__original_version` preserving the declared value. + opened against it, a maintainer approved it nine minutes later, and + **both** suggested fixes were taken rather than just the preferred one — + deepcopy in `__init__` *and* a `declared_version` property preserving the + value the file declared. So #233 is not evidence that maintainers are hostile to outside proposals. It is evidence about what a proposal has to carry. #159 shipped a runnable diff --git a/docs/release-notes/v0.5.0.md b/docs/release-notes/v0.5.0.md index d2af39f..8c971a3 100644 --- a/docs/release-notes/v0.5.0.md +++ b/docs/release-notes/v0.5.0.md @@ -124,11 +124,12 @@ tripwire in the test suite fired on the release with the "behaviour CHANGED" message rather than the routine one, which is the distinction it exists to make. -Two details came out of measuring the release rather than reading the pull -request: the accessor shipped as a public `declared_version` property, not the -`__original_version` the PR described; and `get_global_info()` still returns -the library's spec version, so `iqforge info` correctly keeps printing -`1.0.0 (file); 1.2.6 (reader)`. +The accessor shipped as a public `declared_version` property. The name was +not ours to give: the maintainer's issue comment ended with three candidates — +`.declared_version`, `.file_version`, `.original_version` — and the reply from +here picked the first. One detail did need measuring rather than reading, +though: `get_global_info()` still returns the library's spec version, so +`iqforge info` correctly keeps printing `1.0.0 (file); 1.2.6 (reader)`. Nothing in `iqforge` changed — `load()` reads `core:version` before handing the dict over, which is right under both behaviours — and the `sigmf>=1.11.1` floor diff --git a/tests/test_io.py b/tests/test_io.py index 03a3f47..04679ba 100644 --- a/tests/test_io.py +++ b/tests/test_io.py @@ -319,10 +319,11 @@ def test_annotation_ending_exactly_at_the_end_is_not_flagged(tmp_path: Path) -> #: sigmf releases verified NOT to mutate it. The fix is sigmf-python#160, #: released in 1.13.0: `__init__` deep-copies the metadata, and the value the -#: file declared is kept on a public `declared_version` property. The pull -#: request called that attribute `__original_version`; it shipped under a -#: different name, which is why membership here is earned by measuring an -#: installed release rather than by reading an upstream diff. +#: file declared is kept on a public `declared_version` property. Membership +#: here is earned by measuring an installed release rather than by reading an +#: upstream diff -- not because the diff was misleading, but because the half +#: of the fix that matters downstream is invisible in it: `get_global_info()` +#: still returns the library's spec version, which no changelog line said. FIXED_SIGMF_VERSIONS: frozenset[str] = frozenset({"1.13.0"})