Skip to content

Fix csv_to_wfdb docstring regarding when and where files are written - #591

Merged
tompollard merged 1 commit into
MIT-LCP:mainfrom
cbrnr:fix-csv-to-wfdb-docstring
Oct 31, 2026
Merged

tompollard merged 1 commit into
MIT-LCP:mainfrom
cbrnr:fix-csv-to-wfdb-docstring

Conversation

@cbrnr

@cbrnr cbrnr commented Oct 29, 2026

Copy link
Copy Markdown
Collaborator

Fixes the csv_to_wfdb docstring (see #590):

  • Files are written unless record_only=True (not only if write_dir is set).
  • With the default write_dir="", files are written to the current working directory, not the directory of the input file.
  • The function returns None when writing, and a Record (never a MultiRecord) when record_only=True.

Docstring only, no code changes.

@cbrnr

cbrnr commented Oct 29, 2026

Copy link
Copy Markdown
Collaborator Author

However, while fixing these docstring issues, I found several issues within that module (just by reading the code, so I didn't actually test):

  • dat_file_name is ignored in both modes. It is overwritten with <record_name>.dat.
  • samps_per_frame is dropped when writing. wrsamp accepts it, but it isn't passed through.
  • counter_freq, base_counter, skew and byte_offset are dropped when writing. wrsamp has no matching parameters. adc_res, adc_zero, init_value, checksum and block_size are recomputed by wrsamp.
  • When adc_zero is a list, the default baseline becomes a list of lists.
  • With record_only=True, checksum is computed from the physical values, not the digital samples, and init_value is only an approximation.
  • Broadcasting uses type(x) is int, so NumPy scalars and floats such as adc_gain=200.5 aren't broadcast. An explicit 0 is treated as missing.
  • Several parameter descriptions were copied from other places: file_name from rdrecord, fs ("250 assumed", but it's required) and others from the header spec. The docs also list the default signal names as ch_1, ch_2, but the code produces ch_0, ch_1.
  • In the same file, csv2ann is untested and absent from the docs. It also truncates paths containing dots (./x.csv becomes "") and has no write_dir.

@tompollard tompollard left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this looks good, and thanks for the careful read of the rest of the module.

I've opened issues for the points you raised:

  • #592: csv_to_wfdb ignores several parameters when writing
  • #593: defaults, scalar broadcasting and checksum handling
  • #594: remaining incorrect parameter descriptions in the docstring
  • #595: csv2ann path handling, missing write_dir, tests and docs

@tompollard
tompollard merged commit ff4a7e2 into MIT-LCP:main Oct 31, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants