Skip to content

Conversation

@mbyrnepr2
Copy link
Contributor

@mbyrnepr2 mbyrnepr2 commented Nov 3, 2025

@mbyrnepr2 mbyrnepr2 changed the title wave.py - Allow opening of path-like files. gh-140952: Allow opening of path-like files with wave.open Nov 3, 2025
@hugovk hugovk added the stdlib Standard Library Python modules in the Lib/ directory label Nov 3, 2025
@mbyrnepr2 mbyrnepr2 closed this Nov 3, 2025
@mbyrnepr2 mbyrnepr2 reopened this Nov 4, 2025
@mbyrnepr2 mbyrnepr2 changed the title gh-140952: Allow opening of path-like files with wave.open gh-75593: Allow opening of path-like files with wave.open Nov 4, 2025
@python-cla-bot
Copy link

python-cla-bot bot commented Nov 4, 2025

All commit authors signed the Contributor License Agreement.

CLA signed

@mbyrnepr2
Copy link
Contributor Author

mbyrnepr2 commented Nov 4, 2025

TODO: Docs may need an update.

@mbyrnepr2 mbyrnepr2 marked this pull request as ready for review November 4, 2025 12:47
Lib/wave.py Outdated
def __init__(self, f):
self._i_opened_the_file = None
if isinstance(f, str):
if isinstance(f, (str, os.PathLike)):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As was suggested in #75593 (comment), it is easy to add support of bytes.

def test_open_pathlike(self):
"""It is possible to use `wave.read` and `wave.write` with a path-like file"""
with tempfile.NamedTemporaryFile(delete_on_close=False) as fp:
WAV_FILE = pathlib.Path(fp.name)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use test.support.os_helper.FakePath instead of pathlib.Path. It should support arbitrary Path-like objects.

Also, test with a Path-like object wrapping a bytes path: FakePath(os.fsencode(...)).

.. function:: open(file, mode=None)

If *file* is a string, open the file by that name, otherwise treat it as a
If *file* is a string or a :term:`path-like object`, open the file by that name, otherwise treat it as a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Too long line.

Add also a versionchanged directive.

@mbyrnepr2 mbyrnepr2 force-pushed the wave_open_a_pathlib_file branch from eb1f0ab to ba5ed6f Compare November 11, 2025 12:39
with tempfile.NamedTemporaryFile(delete_on_close=False) as fp:
cases = (
FakePath(fp.name),
FakePath(os.fsencode(fp.name)),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test also for os.fsencode(fp.name).

self.assertIsNone(cm.unraisable)


class WaveOpen(unittest.TestCase):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't there a place for this test in an existing class? You can also look in audiotests -- if this was added before removing aifc, the test should be added there.

Copy link
Contributor Author

@mbyrnepr2 mbyrnepr2 Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had also considered adding the new tests to MiscTestCase or WaveLowLevelTest - I don't have a good reason to create a new class now that I think of it, except that perhaps they don't strictly belong in either of those places.

Just to clarify your point about adding the tests to audiotests.py - are you saying that the new tests should simply be moved there instead? Or do you mean that the existing tests in test_wave.py should inherit the new tests similar to the existing design?
My rationale for adding the tests to test_wave.py is I did feel like they belonged in test_wave.py and not audiotests.py since the classes in audiotests.py are inherited by the classes in test_wave.py to assert a range of things which felt redundant for the new tests to also perform.


If *file* is a string, open the file by that name, otherwise treat it as a
file-like object. *mode* can be:
If *file* is a string or a :term:`path-like object`, open the file by that name,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or a bytes object.

Added support for unseekable files.

.. versionchanged:: 3.15
Added support for :term:`path-like objects <path-like object>`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and bytes objects.

- More accurate documentation.
- More complete test cases.
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. 👍

@@ -0,0 +1 @@
Add support of :term:`path-like objects <path-like object>` in :func:`wave.open`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And bytes objects.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh sorry for that again 😅. I’ll update later today.

@serhiy-storchaka serhiy-storchaka merged commit 3590826 into python:main Nov 12, 2025
46 checks passed
@serhiy-storchaka
Copy link
Member

Thank you for your contribution @mbyrnepr2.

@mbyrnepr2
Copy link
Contributor Author

It was a pleasure and thanks for your time and guidance @serhiy-storchaka

@mbyrnepr2 mbyrnepr2 deleted the wave_open_a_pathlib_file branch November 12, 2025 10:40
StanFromIreland pushed a commit to StanFromIreland/cpython that referenced this pull request Dec 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stdlib Standard Library Python modules in the Lib/ directory

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants