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/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`, 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"})