wfdb-python icon indicating copy to clipboard operation
wfdb-python copied to clipboard

Better encapsulation of record metadata

Open cx1111 opened this issue 2 years ago • 2 comments

Right now we have the Record class with a huge number of attributes in the top level object. These attributes are varied, fall into many categories, and become very hard to keep track of.

I also find it rather awkward that there is no separate 'header' or 'metadata' type of object. ie. rdheader and rdrecord both return the same type of object.

Initial idea:

  • RecordInfo class for storing all the header data. Includes record and signal specification fields, and comments. rdheader will create this type.
  • Same Record class for WFDB records. The info attribute will be a RecordInfo object.

Having the top level p_signal, d_signal attributes is not my favorite, but I feel like it's rather pointless to have another object to capture these fields.

Open to suggestions.

cx1111 avatar May 03 '22 06:05 cx1111

I agree that some restructuring of the Record class would be helpful: https://github.com/MIT-LCP/wfdb-python/blob/3c88d19c32c7a6eb57ec6e8b5c2269693062ac2f/wfdb/io/record.py#L575

RecordInfo class for storing all the header data. Includes record and signal specification fields, and comments. rdheader will create this type.

It would be good to chat about this. I'm getting myself slightly confused about how these classes (e.g. RecordInfo for header information and Record for data) would relate to the files themselves (e.g. x.hea for headers and x.dat for data).

Having the top level p_signal, d_signal attributes is not my favorite, but I feel like it's rather pointless to have another object to capture these fields.

Side note, but it would be nice if these variables had friendlier names. p_signal, d_signal, e_p_signal, and e_d_signal are pretty opaque.

tompollard avatar Jun 01 '22 15:06 tompollard

Related discussion in #376 about signals. Agree with providing friendlier signal attributes (or none at all, see the other issue).

The workflow would be demonstrated in the draft PR #388, which I've just left a comment on.

cx1111 avatar Jun 10 '22 18:06 cx1111