FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-94991 Make `wave._wave_params` public -> `wave.wave_params` by f4hy · Pull Request #94992 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

gh-94991 Make wave._wave_params public -> wave.wave_params - #94992

Open
f4hy wants to merge 4 commits into
python:mainfrom
f4hy:wave/make_wave_params_public
ConversationCommits (4)ChecksFiles changed
Open

gh-94991 Make wave._wave_params public -> wave.wave_params#94992
f4hy wants to merge 4 commits into
python:mainfrom
f4hy:wave/make_wave_params_public

Conversation

f4hy commented Jul 19, 2022 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown

This namedtuple is the public interface for Wave_write.setparams, making this private does not allow users to create an input that is typechecked to Wave_write.setparams.

If this were public users could call with more readable code such as:

import wave
with wave.open("test.wav", "wb") as wav_file:
    params = wave.wave_params(
        nchannels=1, sampwidth=2, framerate=16_000, nframes=0, comptype="NONE", compname="NONE",
    )
    wav_file.setparams(params)

rather than the less readable

import wave
with wave.open("test.wav", "wb") as wav_file:
    params = (1, 2, 16_000, 0, "NONE", "NONE")
    wav_file.setparams(params)

This is even worse in a typed environment where the private type of wave._wave_params is the only way to call setparams which use of private values from the stdlib should be avoided.

Make _wave_params public -> wave_params
35cc786 
This namedtuple is the public interface for Wave_write.setparams,
making this private does not allow users to create an input that is
typechecked to Wave_write.setparams.

Is there a reason to want to hide this from users to make it less
readable for how to call Wave_write.setparams ?

ghost commented Jul 19, 2022 •
edited by ghost
Loading

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

bedevere-bot commented Jul 19, 2022

Copy link
Copy Markdown

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

f4hy mentioned this pull request Jul 19, 2022
Closed
Akuli reviewed Jul 19, 2022
View reviewed changes
Comment thread Lib/wave.py

mbeijen commented Feb 18, 2026

Copy link
Copy Markdown
Contributor

Hi, this seams very reasonable and I think it should be merged in python!

I created a small drop-in replacement for stdlib wave: newwave, available on pypi. I added this PR to that package, thanks!!

But I also added wave.write() as 'syntactic sugar' on top of `wave.open(..., 'wb'):

# Using the convenience function
with newwave.write('output.wav', channels=2, sampwidth=2, framerate=44100) as w:
    w.writeframes(audio_data)

That is even easier!! What do you think?

mbeijen mentioned this pull request Mar 13, 2026
Merged

encukou commented Mar 23, 2026

Copy link
Copy Markdown
Member

This could go together with adding a non-positional format attribute, as needed by #60729.

But I also added wave.write() as 'syntactic sugar' on top of `wave.open(..., 'wb'):

I wouldn't mind this in wave.Wave_write.__init__ itself.
Or if we're doing better user-facing names, prehaps wave.Writer?

github-actions Bot commented May 11, 2026

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

github-actions Bot added the stale Stale PR or inactive for long period of time. label May 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
Akuli Akuli left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

awaiting review stale Stale PR or inactive for long period of time.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL