wfdb-python
wfdb-python copied to clipboard
Fix downloading files in windows
Adding a small change to make optional the use of multiple threads for download files. We couldn't make it work in a conda environment in windows.
With this change, default is the same, but multithread download can be disabled by optional parameter.
Thanks!
As I said before, did you try simply changing "multiprocessing.Pool" to "multiprocessing.dummy.Pool"? I know that multiprocessing is messy and best avoided here, but multiprocessing.dummy should not have the same drawbacks (it has other drawbacks instead.)
If multiprocessing.dummy.Pool doesn't work for you either, that's a bigger problem and something I'd like to try to understand.
I think we can merge in this change after the requested changes are made. It seems useful in general.
Let's figure out #306 separately.
I think we can merge in this change after the requested changes are made. It seems useful in general.
Let's figure out #306 separately.
If we do want to make parallel downloads optional, I'd say the name of the argument should be parallel
or something like that. If we change the implementation to use threads instead of processes then use_multiprocess
becomes misleading.
I'm not opposed to adding that option, I just don't want to add the option solely to avoid a known bug in the package, instead of just fixing the bug.
I also don't understand this change, which seems unrelated:
@@ -4525,7 +4527,8 @@ def dl_database(db_dir, dl_dir, records='all', annotators='all',
db_dir = posixpath.join(dir_list[0], get_version(dir_list[0]), *dir_list[1:])
else:
db_dir = posixpath.join(db_dir, get_version(db_dir))
- db_url = posixpath.join(download.PN_CONTENT_URL, db_dir) + '/'
+
+ db_url = posixpath.join(download.PN_CONTENT_URL, db_dir)
# Check if the database is valid
_url.openurl(db_url, check_access=True)
I think this is probably a mistake, we do want to request a path ending in slash (the path with the slash is the "real" URL, without the slash is a redirection.) I don't remember off the top of my head if check_access=True will treat a redirected URL as valid or not.
Yes, let's set the param name toparallel
.
And I agree that this change isn't meant as a way to avoid solving the actual issue.