-
Notifications
You must be signed in to change notification settings - Fork 2
Document limitations for loading Draeger data #341
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| """Load EIT data from path(s). | ||
| Current limitations: | ||
| - Dräger data is assumed to have a sample frequency of 20 Hz. The sample frequency should be provided while loading |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels a bit "unsafe".
- Dräger data is assumed to have a sample frequency of 20 Hz. The sample frequency should be provided while loading if it differs from 20 Hz.
Maybe we should not give a default frequency at all?
- Dräger data is assumed to have a limited set of (Medibus) data. Newer additions that add data like pleural pressure are not yet supported.
Is there a way to throw an error message or warning if unsupported data is attached? Or a clear error message if unsupported data is tempted to be accessed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've addressed both issues and updated the tests accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the top issue can be removed here.
the assumption no longer holds true, right? It also does not feel like a "limitation" any more.
0b48ab4 to
9d12be1
Compare
| f"File size {file_size} of file {path!s} not divisible by {_FRAME_SIZE_BYTES}.\n" | ||
| f"Make sure this is a valid and uncorrupted Dräger data file." | ||
| "Currently this package does not support loading files containing " | ||
| "esophageal pressure or other non-standard data. " | ||
| "Make sure this is a valid and uncorrupted Dräger data file." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this clearer? Also is it worthwhile explicitly stating to re-save files excluding those data and trying again?
| f"File size {file_size} of file {path!s} not divisible by {_FRAME_SIZE_BYTES}.\n" | |
| f"Make sure this is a valid and uncorrupted Dräger data file." | |
| "Currently this package does not support loading files containing " | |
| "esophageal pressure or other non-standard data. " | |
| "Make sure this is a valid and uncorrupted Dräger data file." | |
| f"<{path!s}> cannot be loaded due to an incompatible file size.\n" | |
| "Make sure this is a valid and uncorrupted Dräger data file.\n" | |
| "Note that this package does not currently support loading" | |
| "files containing esophageal pressure or other non-standard data." |
|
are you pointing this PR at main rather than develop on purpose? Seems unlikely given that you've rebased or merged the branch on develop. |
9d12be1 to
fd30980
Compare
#192#193 and #217 explain two limitation of loading Draeger data. This documents these.