Skip to content
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

Use a single path argument (and deprecate pn_dir and file_name) #534

Open
tompollard opened this issue Apr 2, 2025 · 2 comments
Open

Use a single path argument (and deprecate pn_dir and file_name) #534

tompollard opened this issue Apr 2, 2025 · 2 comments

Comments

@tompollard
Copy link
Member

Currently the WFDB API uses 2 arguments to specify the location of files: pn_dir and file_name. This is confusing for users and confusing for developers (e.g. see: https://github.com/MIT-LCP/wfdb-python/pull/523/files).

I think the reason for having two arguments is to support auto-detection of files (e.g. get locally if local and if not get from the PhysioNet servers). This kind of behaviour doesn't seem like a good idea. If WFDB is retrieving a remote file, it should be explicit.

My preference would be to:

  • Introduce a new path argument
  • Deprecate pn_dir and file_name
  • Remove any logic that automatically retrieves files from remote servers.
@bemoody
Copy link
Collaborator

bemoody commented Apr 3, 2025

For most APIs, I think, the first argument is called record_name. Not file_name, because it isn't a file name.

get locally if local and if not get from the PhysioNet servers

Where do you see that happening? I don't think that's right or at least not intended.

@tompollard
Copy link
Member Author

Where do you see that happening? I don't think that's right or at least not intended.

This is based on my recollection of discussing the project with Chen when the project started. I may well be wrong! I'll spend a bit more time thinking about this (but the original point still stands...I think we should clean up the API).

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

No branches or pull requests

2 participants