Repository navigation
Fix record name and add write_dir in csv2ann - #599
Merged
Merged
Conversation
Take the record name from the CSV file name with os.path.splitext, as csv_to_wfdb does, so paths with dots no longer truncate it. Add a write_dir argument, tests, and list csv2ann in the API docs.
Member
|
Thanks, this looks great! On your question: let's keep the new behaviour in this PR (write to Could you update the changelog to reflect that? Something like: **Fix record name and add write directory in csv2ann**
``csv2ann`` now takes the record name from the CSV file name without its directory and extension, as ``csv_to_wfdb`` does. Paths with a
directory (such as ``data/x.csv``) used to fail when writing, and ``./x.csv`` gave an empty name. The annotation file is written to the new
``write_dir`` argument, which defaults to the current directory. ``csv2ann`` is now included in the API documentation.Happy to merge once that's updated. |
Contributor
Author
|
Thanks, that makes sense. I hadn't noticed that |
Member
Didn't want to nitpick this - thanks! |
tompollard
added a commit
that referenced
this pull request
Oct 31, 2026
Closes #592, closes #593, closes #594. Following the suggestion in #592, `csv_to_wfdb` now builds one `Record` in both modes. With `record_only=False` it writes that record with `Record.wrsamp(write_dir=...)`; with `record_only=True` it returns it. **#592: parameters that had no effect** - `dat_file_name` is used if given (the default is still `<record_name>.dat`). - `counter_freq`, `base_counter`, `skew`, `byte_offset`, `adc_res`, `adc_zero` and `block_size` are written to the header (`byte_offset` also writes the leading bytes, as `wr_dat_file` already does). - `samps_per_frame`, `init_value` and `checksum` follow from the CSV samples, so as suggested they're deprecated: passing them gives a `DeprecationWarning` and they're ignored. (`samps_per_frame` other than 1 also never reached the header before, because `wrsamp` writes `d_signal` unexpanded, see #333.) **#593: defaults and broadcasting** - New `_per_channel` helper: any `str`, `numbers.Number` or NumPy scalar is repeated for all channels, so `adc_gain=200.5` and `baseline=np.int64(3)` work. - Defaults are applied only when the argument is `None`, so `baseline=0` with `adc_zero=1024` stays 0. - `baseline` defaults to `adc_zero` per channel, so `adc_zero=[1, 2]` gives `baseline=[1, 2]` instead of a list of lists. - `init_value` and `checksum` are computed from the digital samples (`Record.adc()` / `calc_checksum()`) in both modes, and the remaining defaults come from `set_defaults()` as in `wrsamp`. **#594: docstring** - Corrected `file_name`, `fs`, `header` (`ch_0, ch_1, ...`) and the descriptions copied from the header spec, and documented the defaults and deprecations. **Compatibility** - With default arguments, the written `100.hea` and `100.dat` for `sample-data/100.csv` are byte-identical to `main`. - With `record_only=True`, the returned record now matches what would be written: `adc_res` defaults to the resolution of `fmt` (16 for the default format, was 12), `init_value` is the rounded digital value (was truncated), and `checksum` is over the digital samples (was over the physical values). The returned record still has `d_signal=None`, as before. **Tests** Seven new tests in `TestCsvToWfdb`: record vs written header, scalar broadcasting, zero and list defaults, parameters reaching the files, and the three deprecated arguments. All seven fail on `main`. `ruff check` is clean, the docs build passes with `-W`, and `pytest -m "not network"` passes (89 tests). This touches the same file as #599 (`csv2ann`), but a different function. Both add a changelog entry at the same place and append tests to `tests/io/test_convert.py`, so whichever is merged second will have small conflicts there. I'll rebase it.
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.
Closes #595.
os.path.splitext(os.path.basename(file_name))[0], the same as incsv_to_wfdb. Before,file_name.split(".")[0]turned./x.csvinto""anddata/v1.2/x.csvintodata/v1.write_dirargument (default"", the current directory), passed towrann. It's added at the end of the signature so positional calls keep working../x.csv,data/v1.2/x.csv). The two tests that exercise the fix fail onmain.csv2annis added todocs/convert.rst. The docs build passes with-W.Behaviour change: the annotation file used to be written next to the CSV file (for paths without other dots). Now it's written to
write_dir, likecsv_to_wfdb. That follows the note in #595 that both functions should write to the same place. If you'd prefer to keep the old location as the default, I can makewrite_dir=Nonemean "next to the CSV file" instead.ruff checkis clean andpytest -m "not network"passes (85 tests).