Skip to content

IO updates#416

Merged
bmcfee merged 3 commits intomainfrom
loader-path-support
Feb 21, 2025
Merged

IO updates#416
bmcfee merged 3 commits intomainfrom
loader-path-support

Conversation

@bmcfee
Copy link
Copy Markdown
Collaborator

@bmcfee bmcfee commented Feb 21, 2025

Fixes #415
Fixes #413


This PR adds support for pathlib.Path objects in all mir_eval.io loaders. (Easy fix, just adding a case to the _open context manager's type check.)

I've added one unit test that hits load_delimited with either string or Path input. I think this is sufficient since: A) almost all other loaders run through load_delimited, and B) those that don't use the same _open context manager anyway.

Since I was in .io anyway, I added a deprecation on load_wav.

Assuming this passes CI, I think it should be good to merge.

@bmcfee bmcfee added this to the 0.8.1 milestone Feb 21, 2025
@codecov
Copy link
Copy Markdown

codecov bot commented Feb 21, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Flag Coverage Δ
unittests 85.67% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
mir_eval/io.py 92.04% <100.00%> (+0.09%) ⬆️

@bmcfee
Copy link
Copy Markdown
Collaborator Author

bmcfee commented Feb 21, 2025

Rewrote this a bit following #413 (comment) ; I'm not wild about raising an IOError from a TypeError, for many reasons. (It's obtuse, provides no additional information, and adds complexity. It also doesn't do anything useful if open throws a different kind of error.)

I've done it here so that we preserve the type of exception raised in case of failure, as changing it would constitute API breakage and I'd rather not do that in a minor release (as this is slated for). But going forward (0.9), we should rip out this indirection and just let open() throw whatever exception it likes back up the stack.

@bmcfee bmcfee merged commit 5209a70 into main Feb 21, 2025
@bmcfee bmcfee deleted the loader-path-support branch February 21, 2025 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deprecate io.load_wav Support Path objects in data loaders

1 participant