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

Add option to loop through multiple waveforms when running benchmarking #36

Merged
merged 3 commits into from
Apr 8, 2024

Conversation

briangow
Copy link
Collaborator

@briangow briangow commented Apr 5, 2024

This update gives the option to run the benchmarking code ( #12 ) on multiple waveforms. It also updates the input arguments to set the help option and indicate which arguments are required when running against a single waveform versus running against multiple waveforms. An upcoming PR will print out the filename being run.

@tompollard
Copy link
Contributor

It would be cleaner just to allow the input to be an individual file OR a folder. Does it need to be more complicated than this?

@briangow
Copy link
Collaborator Author

briangow commented Apr 6, 2024

@tompollard , I was planning to allow a benchmarking run which pulls files from multiple PhysioNet databases. Having said that, it would be more efficient to simply download the files and they could be placed into a single folder, so that might not be necessary.

@briangow
Copy link
Collaborator Author

briangow commented Apr 8, 2024

@tompollard , I decided to stick with the design that I had because I also need to run benchmarking across multiple file formats. This will be much easier to automate with a control file.

@briangow briangow marked this pull request as ready for review April 8, 2024 13:18
@tompollard
Copy link
Contributor

tompollard commented Apr 8, 2024

@tompollard , I decided to stick with the design that I had because I also need to run benchmarking across multiple file formats. This will be much easier to automate with a control file.

Up to you, and I know you are working to a tight timeline. I think it makes the package unnecessarily fiddly! It isn't clear to me why having to pass a CSV as an argument is easier than just pointing to a folder of well-organised files. If it is necessary to specify details beyond those captured in the files, then these details could be captured in a metadata file in the folder.

@tompollard tompollard merged commit fd2b0f3 into main Apr 8, 2024
1 check passed
@tompollard tompollard deleted the bg_loop_benchmarking branch April 8, 2024 14:03
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

Successfully merging this pull request may close these issues.

2 participants