Add cross-version serialization tests#4636
Open
h-mayorquin wants to merge 9 commits into
Open
Conversation
Generate a battery of canonical objects (recordings, a preprocessing chain, a probe recording including an interleaved multi-probe case, and a sorting) under an old spikeinterface release, then load them with the current code and assert their content. Catches changes that silently break loading of old saved files: moved classes, changed constructor signatures, and changed on-disk encodings, across json/pickle recipes and folder/zarr materialized state. Live generation via an isolated uv environment (nothing committed): per-PR runs against the previous release, a weekly matrix covers 0.102-0.104. The version floor is the oldest release that installs cleanly on the CI Python.
Add workflow_dispatch for post-merge manual runs, and temporarily let the weekly matrix run on pull_request to exercise it once in CI. The pull_request trigger on the matrix job must be removed before merge.
…ger) The full version matrix passed once in CI on this PR; remove the temporary pull_request trigger so it no longer runs the heavy matrix on every push. It now runs weekly and via manual workflow_dispatch (available post-merge).
…ity_testing' into backwards_comptability_testing
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
2 participants
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.
SpikeInterface has no test that loads an object serialized by an older release with the current code, even though that is exactly the path that breaks users. We only round-trip within a single version, which never exercises a class move, a renamed constructor argument, or a change in how something is written to disk. Two recent cases make the gap concrete. On #4588, Sam pointed out that moving
NoiseGeneratorRecordingout ofcorecould break any old JSON or pickle that stored its fully qualified class path. The probe refactor in #4465 changes how a probe is written to disk (contact_vectorproperty to aProbeGroupobject), which risks old saved recordings either failing to load or, worse, silently mis-mapping channels to locations. Nothing in continuous integration would catch either today.This adds a harness that generates a small battery of canonical objects under an old release and loads them with the branch, asserting content rather than only that they load. Fixtures are regenerated from a real old install each run in an isolated uv environment, so nothing is committed to the repository. The battery covers both failure axes: JSON recipes catch moved classes and changed signatures (the #4588 case, where the fixture records the old
core.generate.NoiseGeneratorRecordingpath and loading exercises the redirect), while folder and zarr fixtures catch on-disk encoding changes. Those include a single-probe and an interleaved multi-probe recording that assert exact per-channel locations, so a silent channel-to-location scramble (the case that motivated_global_contact_orderin #4465) fails the test instead of passing on shape alone. It runs against the previous release on every pull request touchingcore, and the full version matrix weekly.I set the version floor at 0.102 because 0.100 and 0.101 no longer install cleanly on the continuous-integration Python (numcodecs dependency rot). This makes "testable floor equals support floor" a deliberate choice worth confirming with the team rather than an accident.