GH-132445: Allowing to reset parameters of Wave_write by ygerlach · Pull Request #132448 · python/cpython · GitHub

This allows to re-set a parameter of Wave_write to the same value without causing an Error. This allows to implemet repeated writes to a wav file without special care for the first write.

Issue: Wave_write throws an error if a setter to an written header information is set to the same value #132445

All commit authors signed the Contributor License Agreement.

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

I'm still not convinced but in any case, if this feature is to be accepted, we need tests and a NEWS entry, as well as a doc change.

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

I have made the requested changes; please review again.

Thanks for making the requested changes!

@picnixz: please review the changes made to this pull request.

TBH, I am neutral on this addition. I can understand that it's annoying for people passing around wave objects and forcibly changing the parameters, but OTOH I still believe it's incorrect to forcibly reset a parameter even if it's the same. Users should instead check the current value itself, and if it's not yet set, they should wrap it in a try-except call.

To ease their life, we could however expose the number of bytes that were already written so that they don't need a try-except but just something like:

if self.has_data:
    assert self.getnchannels() == EXPECT_NCHANNELS
else:
	self.setnchannels(EXPECT_NCHANNELS)

It could be annoying but I think it's more an API misuse rather an API flaw. For your use case, I think the correct way to do it is to use synthesize_stream_raw() or to have a merge_and_synthesize function which

for s in piper.synthesize_stream_raw(...):
	wav_file.writeframes(s)

So, while I have reviewed your code, I don't really think we'll make the change in the stdlib itself. Another reason is that it makes maintainability harder (if we add more setters in the future, it could be annoying if some values are inter-dependent as well).

@serhiy-storchaka should decide whether or not we accept this change.

Suggested change

      
                  Raises an :exc:`wave.Error`, if the value is set after data was written.
          
                  Raise :exc:`wave.Error` if a different value is set after data was written.

Suggested change

      
                  .. versionchanged:: 3.14
          
                     :exc:`wave.Error` is not raised, if the value is the same.
          
                  .. versionchanged:: next
          
                     No :exc:`wave.Error` is raised if the value does not change.

Same for the rest of the docs.

We also need to test rounded values.

The int(round(framerate)) should be tested when two distinct input framerates give the same rounded framerate.

I noticed that before this change we could write self.setframerate(0.5) and the framerate would be set to 0. With this change, we prevent a bad framerate.

So also test that self.setframerate(0.5) raises an exception, but not self.setframerate(0.51).

I created #144975 for that!

@serhiy-storchaka should decide whether or not we accept this change.
I'm not that person, but I also do not really think this is a good idea

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

Closing because the issue has been closed as not planned.

bedevere-app Bot added the awaiting review label Apr 12, 2025

bedevere-app Bot mentioned this pull request Apr 12, 2025

Wave_write throws an error if a setter to an written header information is set to the same value #132445

Closed

ygerlach force-pushed the wave_parameters branch from b8f4980 to c55c503 Compare April 23, 2025 21:32

picnixz requested changes Apr 24, 2025

View reviewed changes

bedevere-app Bot added awaiting changes and removed awaiting review labels Apr 24, 2025

ygerlach force-pushed the wave_parameters branch from c55c503 to 4fd6c81 Compare April 30, 2025 16:22

pythonGH-132445 : Allowing to reset parameters of Wave_write

8b77c5a

ygerlach force-pushed the wave_parameters branch from 4fd6c81 to 8b77c5a Compare April 30, 2025 16:25

bedevere-app Bot added awaiting change review and removed awaiting changes labels Apr 30, 2025

bedevere-app Bot requested a review from picnixz April 30, 2025 16:26

picnixz reviewed May 9, 2025

View reviewed changes

picnixz requested a review from serhiy-storchaka May 9, 2025 11:06

mbeijen mentioned this pull request Feb 18, 2026

wave: setframerate() accepts invalid frame rate values that round to zero #144975

Closed

github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 23, 2026

picnixz closed this Jul 3, 2026

+                      if self._datawritten and self._framerate != rounded_framerate:
                           raise Error('cannot change parameters after starting to write')
-                      if framerate <= 0:
+                      if rounded_framerate <= 0:


		Set the number of channels.

		Raises an :exc:`wave.Error`, if the value is set after data was written.

	Raises an :exc:`wave.Error`, if the value is set after data was written.
	Raise :exc:`wave.Error` if a different value is set after data was written.

		.. versionchanged:: 3.14
		:exc:`wave.Error` is not raised, if the value is the same.

+                              writer.setnchannels(2)
+                              writer.setsampwidth(2)
+                              writer.setframerate(2)

                   def setframerate(self, framerate):
-                      if self._datawritten:
+                      rounded_framerate = int(round(framerate))

GH-132445: Allowing to reset parameters of Wave_write by ygerlach · Pull Request #132448 · python/cpython · GitHub

Repository navigation

GH-132445: Allowing to reset parameters of Wave_write - #132448

GH-132445: Allowing to reset parameters of Wave_write#132448ygerlach wants to merge 1 commit intopython:mainpython/cpython:mainfrom ygerlach:wave_parametersygerlach/cpython:wave_parametersCopy head branch name to clipboard

Conversation

ygerlach commented Apr 12, 2025 • edited by bedevere-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

python-cla-bot Bot commented Apr 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bedevere-app Bot commented Apr 12, 2025

Uh oh!

bedevere-app Bot commented Apr 23, 2025

Uh oh!

picnixz left a comment

Choose a reason for hiding this comment

Uh oh!

bedevere-app Bot commented Apr 24, 2025

Uh oh!

ygerlach commented Apr 30, 2025

Uh oh!

bedevere-app Bot commented Apr 30, 2025

Uh oh!

picnixz left a comment

Choose a reason for hiding this comment

Uh oh!

picnixz May 9, 2025

Choose a reason for hiding this comment

Uh oh!

picnixz May 9, 2025

Choose a reason for hiding this comment

Uh oh!

picnixz May 9, 2025

Choose a reason for hiding this comment

Uh oh!

picnixz May 9, 2025

Choose a reason for hiding this comment

Uh oh!

picnixz May 9, 2025

Choose a reason for hiding this comment

Uh oh!

mbeijen Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

mbeijen commented Feb 18, 2026

Uh oh!

github-actions Bot commented Apr 23, 2026

Uh oh!

picnixz commented Jul 3, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

GH-132445: Allowing to reset parameters of Wave_write#132448
ygerlach wants to merge 1 commit into
python:mainfrom
ygerlach:wave_parameters

ygerlach commented Apr 12, 2025 •
edited by bedevere-app Bot
Loading

python-cla-bot Bot commented Apr 12, 2025 •
edited
Loading