-
Notifications
You must be signed in to change notification settings - Fork 302
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
Add write_dir argument to csv_to_wfdb. Fixes #67. #492
Conversation
d6d5aa7
to
caff031
Compare
Tests are failing on an unrelated issue, presumably related to an update to Numpy (e.g. this? https://numpy.org/devdocs/release/1.24.0-notes.html#conversion-of-out-of-bound-python-integers) |
It should be The existing logic is pretty broken, though! |
8463fe6
to
d9f4e3c
Compare
No test cases - please add a test case so we can see that this function works. |
d9f4e3c
to
626307a
Compare
626307a
to
e6b3b69
Compare
…r earlier versions. Tests are failing on the test-deb10-i386 build because it is running an old version of Pandas.
@bemoody this is now ready to review. |
|
I considered this but decided to use |
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.
Thanks - looks good!
Thanks Benjamin |
As discussed in #490,
wfdb-python/wfdb/io/convert/csv.py
Line 10 in 34b989e
It's inconvenient not to be able to specify the output directory. This pull request adds a new
output_dir
argument to thecsv_to_wfdb
function. By defaultoutput_dir
is set to None, which will maintain backwards compatibility. Settingoutput_dir
to a directory will mean that output files are saved to this directory.I have set this to a WIP, because I haven't tested the new behaviour (other than running
pytest
). @jshaffer94247, if you have an opportunity to test the fix, I'd appreciate your feedback.