Repository navigation
Conversation
Ivorforce
changed the title
… rdann, in case the file is not read from disk.
Ivorforce
changed the title
|
Update: I have added the same cabability to
|
|
Update 2: I have opened #498. That PR is a more comprehensive and streamlined rewrite, and thus supersedes it, in my opinion. |
|
Thanks for this, and sorry it sat for so long. Reading records from memory (e.g. from an upload widget) is a good use case. Since this PR was opened, the reading code has moved to import fsspec, wfdb
fs = fsspec.filesystem("memory")
fs.pipe("/upload/100.hea", hea_bytes)
fs.pipe("/upload/100.dat", dat_bytes)
record = wfdb.rdrecord("memory://upload/100")I tried this locally after adding Would you be interested in opening a PR for that instead (with tests and a docs note)? If so, I think we can close this one and #498. |
`proc_ann_bytes` and its helpers decode an annotation file by indexing
the Nx2 NumPy byte array one element at a time (`filebytes[bpi, 1]`,
`np.int64(...)`, `.astype("i1")`), and build each aux note with
`"".join(chr(c) for c in ...)`. Each of those makes a NumPy scalar,
which is slow in a Python loop.
This converts the byte pairs to a list once with `.tolist()` and does
the rest with plain ints. Signed `subtype`/`num` bytes are converted
with a small helper, and aux notes are decoded with
`bytes(...).decode("latin-1")`, which gives the same characters as
`chr()` for 0-255. Since no NumPy scalars are involved any more, the
decoding no longer depends on NumPy's scalar type promotion rules, which
caused the `OverflowError` in #493 and #522 (already worked around on
main with `np.int64`).
The output is unchanged: I compared every field (`sample`, `symbol`,
`subtype`, `chan`, `num`, `aux_note`, `fs`, `label_store`, custom
labels, and dtypes) for all 15 annotation files in `sample-data`, with
and without `sampto`, against main.
Benchmark (median, Apple Silicon, alternating runs of main and this
branch; the machine was busy, so the absolute numbers are noisy):
| file | main | this PR | speedup |
| --- | --- | --- | --- |
| `rdann` 100.atr | 4.10 ms | 2.01 ms | 2.0x |
| `rdann` 1003.atr | 6.24 ms | 3.31 ms | 1.9x |
| `rdann` 12726.wabp (many aux notes) | 16.5 ms | 6.4 ms | 2.6x |
| `rdann` 03700181.gqrsl | 2.99 ms | 1.64 ms | 1.8x |
| `rdann`, 100,000 beats with some aux notes (written with `wrann`) |
176 ms | 76 ms | 2.3x |
| `proc_ann_bytes` only, 12726.wabp | 22.4 ms | 7.6 ms | 2.9x |
Tests: added a round-trip test for `subtype`, `chan` and `num` values
(including negative subtypes). `uv run pytest -m "not network"` passes,
`tests/test_annotation.py` passes with network tests, `ruff
format`/`ruff check` are clean, and the docs build without warnings.
Added a changelog entry.
Open PRs #491 and #498 also touch `annotation.py`, but only how the file
is opened and read into bytes, not the decoding, so they shouldn't
conflict beyond nearby lines.
|
Thank you for getting back to this @tompollard. I'm happy to hear it's already almost working out of the box so my larger changes from here are no longer needed :) Closing in favour of #629 |
… in case the file is not read from disk.
This is needed if, for example, files are streamed using a file upload dialog versus read from the file system. Personally, I ran into this limitation using Dash's Upload widget.
There are some basic ToDos:
However, with this PR the code is starting to get a bit messy. I propose the following changes to make it cleaner again (happy to oblige within the PR if accepted):
Nonetest against sig_dataLet me know what you think.