Conversation
The file predates the repo's current ruff config and had never been run through it, so the pre-commit hook reflows it wholesale on first touch. Separated here so the change that follows is readable.
Two things every consumer of a replayed file needs, and only the reader can supply. `chunk_dim` names the dimension messages accumulate along. A consumer caches state against the stream's configuration -- channel count, labels, sample rate -- and must exclude the one dimension whose length is just however much of the file this chunk covered. It cannot reliably infer which that is: it is `time` here but `win` downstream of a windowing stage, so a guess either thrashes on chunk-size jitter or stops noticing real changes. Both iterators declare it, and it holds whether the stream is regular or carries per-sample timestamps. `CoordinateAxis.fingerprint` is a content digest, computed on first access and cached on the instance. Each template builds its `ch` axis once and every message reuses that object, so priming costs one checksum per stream. Left cold it is computed by the first stateful consumer in this process -- and, because unpickling builds a new axis object per message, by the first consumer in every other process, on every message. Unverified by tests: this repo has no XDF fixture, only the placeholder in tests/test_iter.py. The change mirrors ezmsg-neo and ezmsg-nwb, where it is covered. Requires ezmsg 3.10.0b2 for both fields.
`tests/test_iter.py` was a `test_dummy` placeholder with a TODO asking for a small XDF source, so nothing in this package was covered -- including the chunk_dim and fingerprint change in the previous commit. The fixture is generated rather than checked in as a binary, so it stays readable and adjustable: a test needing an irregular stream, a string stream or a different channel layout changes an argument instead of asking someone to produce a new recording. `create_test_xdf.py` writes the XDF chunked format directly, with each field pinned against what pyxdf's reader actually consumes -- `_read_varlen_int`, `_read_chunk3` and the tag dispatch in `load_xdf` -- since that is the only reader these files ever meet. Three choices in the generator are deliberate and would otherwise look arbitrary: * Every sample carries an explicit timestamp rather than relying on delta decompression. The compressed form encodes "same as last plus 1/srate", which would make the fixture silently agree with any reader that got the nominal rate wrong. * Streams start at t=10 s, not 0, so a reader honouring `rezero` is distinguishable from one ignoring it. * ClockOffset chunks are written even though there is no clock skew to model. Without them pyxdf logs "Segments and clock-segments differ" on every load, and a fixture that warns every time trains readers to ignore warnings. The default file has two streams -- a 100 Hz 4-channel float32 ramp and an irregular string marker stream -- split across several sample chunks so the reader's chunk stitching is exercised rather than arriving as one block. 20 tests cover sample values and ordering, chunk_dur, rezero, the nominal rate reaching the axis gain, per-sample timestamps on the irregular stream, force_single_sample, and both fields the previous commit added. A class at the end checks the fixture itself, since a wrong fixture would make every other assertion agree with the wrong thing. Verified by mutation rather than by the tests merely passing: dropping chunk_dim fails 3, dropping the fingerprint priming fails 3, and rebuilding the channel axis per message instead of reusing it fails 2.
The push and pull_request triggers were commented out, leaving workflow_dispatch as the only way to run the suite. That was reasonable when the suite was a single `test_dummy` placeholder, and is not now that it covers the iterators, the producers and both units. Every other ezmsg source package triggers on push to main and on pull requests; blackrock, neo and nwb also include dev, which is where these PRs are based, so that is matched here. Without it a PR to dev reports only the publish workflow's build job, which does not run a single test.
setup-uv is configured with `cache-dependency-glob: "uv.lock"`, but no
uv.lock is committed here -- nor in any sibling ezmsg package. The glob matches
nothing and the action fails the job before a single dependency is installed:
##[error]No file in /home/runner/work/ezmsg-xdf/ezmsg-xdf matched to
[uv.lock], make sure you have checked out the target repository
Latent since the workflow was written, and invisible until the previous commit
turned the triggers on. ezmsg-lsl already keys on pyproject.toml; this matches
it.
ruff has flagged W291 on line 65 since before any of this work. It went unnoticed because the test workflow, which runs the lint step, was never triggered; with the triggers on it fails every matrix entry. Fixing it means touching the file, and this one predates the repo's current ruff config just as iter.py did, so the pre-commit hook reflows it wholesale on first touch. Both are in one commit here because the whitespace fix cannot be staged without the reformat.
This was the only ezmsg source package still on `ez.Unit` + `GenState`;
blackrock, lsl, neo and nwb all use `BaseStatefulProducer` and
`BaseProducerUnit`. Four things follow from joining them.
**The file open no longer blocks the event loop.** `pyxdf.load_xdf` reads and
decodes the entire file, and it ran inside a synchronous `initialize()`, so
graph startup stalled every other unit in the process for as long as that took.
It is now a state reset, and `_areset_state` puts it on a worker thread.
This deliberately diverges from ezmsg-neo and ezmsg-nwb, which call
`_reset_state()` eagerly from `__init__` and therefore still pay the first open
on the loop -- nwb's own test says as much ("Discard the eager sync invocation
from __init__"), so its `_areset_state` only covers reopens after a settings
change. Nothing in this package's public surface reads stream metadata before
the first chunk, so there is nothing to lose by waiting, and a test asserts
both halves: construction touches no file, and the load lands off the loop.
**Settings can change at runtime.** `BaseProducerUnit` brings `INPUT_SETTINGS`
and delegates to `update_settings`; previously settings were read once in
`initialize` and a running unit could not be retargeted. `playback_rate` and
`self_terminating` are listed in `NONRESET_SETTINGS_FIELDS` because they belong
to the unit's pacing rather than to the reader, so changing them must not throw
away a loaded file.
**The two near-identical unit bodies collapse** into `_XDFUnitBase`, which owns
the playback clock and end-of-file handling. Both publishers are named
`produce`, which matters: ezmsg collects publishers per attribute, so keeping
the old `pub_chunk`/`pub_multi` names left `BaseProducerUnit.produce` running
alongside them -- two publishers draining one producer, neither stopping. That
hung the new graph tests until the names matched.
**Templates are built in one place.** `_build_template` and `_with_time`
replace the copy in each iterator, so the chunk_dim declaration and the
fingerprint priming exist once rather than twice.
`XDFIterator` itself is unchanged except for a public `exhausted` property and
`print` becoming `ez.logger.info` -- a library writing to stdout is a defect
regardless.
The keyword API is preserved: `BaseProducer.__init__` forwards `**kwargs` to
the settings type, so `XDFAxisArrayIterator(filepath=..., select=...)` still
works, alongside `settings=`. `XDFMultiIteratorUnitSettings` is kept as an
alias of the settings type that moved to `iter.py`.
27 tests pass, up from 20. The seven new ones cover the producer contract
(lazy open, off-loop load, both construction forms, which settings reopen the
file) and both units end to end in a real `ez.run` graph -- which is what
caught the publisher-name collision.
The async test is driven with `asyncio.run` rather than written `async def`:
the pytest config names `asyncio_mode` but pytest-asyncio is not installed, so
an async test would be silently never awaited and would pass without running.
Declare chunk_dim and prime the channel fingerprint
Build the iterators on ezmsg-baseproc's producer classes
Member
Author
|
@copilot resolve the merge conflicts in this pull request |
Contributor
|
@cboulay I can’t apply these changes because the pull request’s branch is the repository’s default branch, which I’m not allowed to push to directly. You can ask me to create a new pull request with the requested changes instead and I'll open a separate PR on a new branch. |
Member
Author
|
@copilot create a new pull request on dev to resolve the conflicts in this PR. |
Contributor
# Conflicts: # src/ezmsg/xdf/source.py Co-authored-by: cboulay <303797+cboulay@users.noreply.github.com>
[WIP] Fix merge conflicts in stacked pull request
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
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.
No description provided.