Repository navigation
Every public record keeps a read-only copy of its own of the arrays it holds, however it was built - #948
Merged
jmrplens merged 1 commit intoOct 7, 2026
Conversation
jmrplens
added this pull request to stack #950
October 7, 2026 15:17
|
Important Review skippedToo many files! This PR contains 217 files, which is 117 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. ⚙️ Run configuration
📒 Files selected for processing (217)
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Sorry @jmrplens, your pull request is larger than the review limit of 150,000 diff characters |
github-actions
Bot
added
documentation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## fix/subscripts-follow-the-standard #948 +/- ##
======================================================================
- Coverage 96.80% 96.80% -0.01%
======================================================================
Files 443 443
Lines 87889 88037 +148
======================================================================
+ Hits 85082 85223 +141
- Misses 2807 2814 +7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
Numerical conformance
1893/1893 checks pass across 111 domains and 523 standards (227 normative designations, 125 further published sources). Used in the tables below is how much of that clause's published tolerance the deviation consumes: 100 % sits exactly on the limit, 5 % uses a twentieth of the allowance, and a dash means the clause states no two-sided tolerance for the quantity, so there is no budget to spend. It is reported and never used to decide a verdict, which is settled at full precision before any rounding. New checks (2)
Closest to their published limit (top 5) The rows with the least room left, so the ones a change is most likely to push over.
|
| Python Version | Tests | Failures | Coverage | Status |
|---|---|---|---|---|
| macos-latest-3.13 | 26975 | 0 | 96.8% |  Passed |
| macos-latest-3.14 | 26975 | 0 | 96.8% | Passed |
| ubuntu-latest-3.13 | 26975 | 0 | 96.8% | Passed |
| ubuntu-latest-3.14 | 26975 | 0 | 96.8% | Passed |
| windows-latest-3.13 | 26975 | 0 | 96.8% | Passed |
| windows-latest-3.14 | 26975 | 0 | 96.8% | Passed |
Full report at this commit: docs/CONFORMANCE.md · docs/conformance.json · full CI artifacts
jmrplens
force-pushed
the
fix/records-copy-at-construction
branch
from
October 7, 2026 16:51
7f8ffe1 to
b665f7f
Compare
…t holds, however it was built The library's functions already copy the arrays a result keeps, but a record built by hand, by calling its class directly (`room.ImpulseResponseResult(ir=measured, ...)`, or `aircraft.AnpNpdCurves(...)` filled from your own NPD data), still kept the array you handed it, so editing that array afterwards edited the record. With this change every one of the 428 public records that can hold an array copies it at construction, whoever builds it.
jmrplens
force-pushed
the
fix/records-copy-at-construction
branch
from
October 7, 2026 18:05
b665f7f to
835d8e1
Compare
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The library's functions already copy the arrays a result keeps, but a record built by hand, by calling its class directly (
room.ImpulseResponseResult(ir=measured, ...), oraircraft.AnpNpdCurves(...)filled from your own NPD data), still kept the array you handed it, so editing that array afterwards edited the record. With this change every one of the 428 public records that can hold an array copies it at construction, whoever builds it.They all inherit one private base,
OwnsArraysinphonometry._internal.frozen. When the record is built it replaces every array it was given, also inside a tuple, a list or any mapping, by a read-only C-ordered copy of its own, and only then runs the class's own__post_init__, which is composed around the copy rather than replaced, so the checks and normalisations a class already had read the record's copy. Your array keeps itswriteableflag and nothing you do to it afterwards reaches the record. A mapping that holds an array comes back as a plaindict(aMappingProxyTypeas one), never as your own container.A record held whole inside another one is left as it is and answers for its own arrays, and an
io.Signalis no exception. The seven results that hand a waveform back as aSignal(filters.OctaveFilterResult,signals.EnvelopeResult,signals.ResampledSignalResult,signals.AlignedImpulseResponseResult,signals.SynchronousAverageResult,environment.DirectSoundSubtractionandunderwater.PileStrikeResult) hold one of their own when the library builds them, whose samples stay writeable like anySignal's, and hold your own when you build them by hand around it. ASignalhanded to a field that takes an array is copied like any array, and its own samples stay writeable.The functions of the library no longer copy an array they hand to such a record, and the records' own checks no longer copy one either, so no array is copied twice: 340
read_only_copy, 199.copy()and 10.astypecalls are gone.What breaks, and how to migrate. Every array a result holds, apart from the samples of a
Signalit holds, now refuses in-place writes, the arrays it computed as well as the ones it was given; writing into one raisesValueError, so copy it first (edited = result.levels.copy()), as the migration guide shows. A record you build by hand no longer follows later edits of the array you gave it, so fill the array before you build the record. A mapping field that held arrays comes back as a plaindictrather than yourOrderedDict,defaultdictor own mapping class. A record built from a Fortran-ordered or broadcast array holds a C-ordered copy. The migration guide (docs/start/upgrading.md and its two site twins) and the changelog say all of this, and the sentence the previous change wrote about hand-built records now says they copy too.The cost. The copy takes no time worth the name beside the computation: 43 ms for the 269 MB that a 400 by 600 cell FDTD run with a snapshot every ten steps holds, out of 7 s. While the record is being built the solver's arrays and the record's copy are both alive, though, so the peak memory of the largest results grows by their size: from 587 to 855 MB for that FDTD run, from 202 to 333 MB for a 20 km underwater parabolic equation on 4096 depths in 5 m steps, from 207 to 287 MB for an elastic FDTD run on 200 by 500 cells with its snapshots, and from 112 to 139 MB for a 1 kHz atmospheric GFPE field 3 km long and 100 m high. Those are the places where the copy is large.
The gate.
make array-aliasingand its CI job now also fail on a public record that can hold an array and does not inherit the base, and on a copy written around an array handed to one (read_only_copy,np.copy,.astype,read_onlyaround a copy, or.copy()on a field that only holds an array) or made first into a local name that is only read before it is handed over. Run against the tree before this change it fails on 428 records without the base. CONTRIBUTING, the Makefile and the CI job describe the rule.How it was verified. tests/test_owns_arrays.py (42 tests) builds 11 public records by hand from a writeable array, writes into the array and checks the record did not move: before this change all 11 shared memory with the caller, now none does. It also covers the base itself on purpose-built records: tuples, lists, named tuples and every kind of mapping, an
InitVarargument, a class's own and an inherited__post_init__, read-only and strided views of a writeable array, C order, and aSignalheld whole or handed to an array field. tests/test_check_array_aliasing.py (81 tests, 35 new) covers the new rules of the gate, including a helper that writes into its argument through an augmented assignment. Four existing tests expected an array a result computed (a default band axis, a band importance table) to be writeable, two of them by writing into it; they now check that it shares no memory with the published table it came from and refuses writes. The full suite passes on Python 3.13 (26904 passed, 69 skipped), the 7483 documentation snippets over 730 pages run, the site type-checks, builds and validates, the conformance report is unchanged (1893 of 1893 checks), the API reference regenerates with no change, and ruff, ruff format, mypy (688 files) and bandit are clean.