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

gh-105096: Reformat wave documentation #105136

Merged
merged 4 commits into from
May 31, 2023
Merged

gh-105096: Reformat wave documentation #105136

merged 4 commits into from
May 31, 2023

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented May 31, 2023

Add ".. class::" markups in the wave documentation.

  • Reformat also wave.py (minor PEP 8 changes).
  • Remove redundant "import struct": it's already imported at top level.

📚 Documentation preview 📚: https://cpython-previews--105136.org.readthedocs.build/

Add ".. class::" markups in the wave documentation.

* Reformat also wave.py (minor PEP 8 changes).
* Remove redundant "import struct": it's already imported at top
  level.
@vstinner
Copy link
Member Author

vstinner commented May 31, 2023

@hugovk: I wrote this doc change to help me fixing PR #105098 doc warnings :-)

The :mod:`wave` module provides a convenient interface to the WAV sound format.
Only PCM encoded wave files are supported.
The :mod:`wave` module provides a convenient interface to the Waveform Audio
"WAVE" (or "WAV") file format. Only uncompressed PCM encoded wave files are
Copy link
Member Author

Choose a reason for hiding this comment

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

The wave.py file only uses "WAVE" name, whereas the doc only uses "WAV" name. The Wikipedia article is called "WAV" and says that the format is called "WAVE". The Python module is called "wave". I'm confused :-) https://en.wikipedia.org/wiki/WAV

@hugovk
Copy link
Member

hugovk commented May 31, 2023

Nice, just three Sphinx warnings left:


Warning: py:meth reference target not found: close
Warning: py:meth reference target not found: get*
Warning: py:meth reference target not found: set*

For the first one, I think we can just add an exclamation mark, because I don't think we know which close it is in this context?

For the last two, we can either add an exclamation mark (for example, :meth:!get*) or list the actual methods, but there's quite a lot, so !` is good.

When there's no longer any warnings in wave.rst, we can remove it from Doc/tools/.nitignore, then the CI won't allow new warnings back into wave.rst.

@hugovk
Copy link
Member

hugovk commented May 31, 2023

Oh and let's backport this to 3.11, I'll add the labels.

@hugovk hugovk added needs backport to 3.11 only security fixes needs backport to 3.12 bug and security fixes labels May 31, 2023
@vstinner
Copy link
Member Author

Nice, just three Sphinx warnings left:

Interesting. I tried to fix them.

For the last two, we can either add an exclamation mark

I used the lazy fix: I removed :meth: markup and use preformatted text instead.

I will wait for your new review. Tell me if you want further changes. The new code looks good: https://cpython-previews--105136.org.readthedocs.build/en/105136/library/wave.html#wave-read-objects

I'm not excited by wave.Wave_write documentation, but I was too lazy to come up with something better.

Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

Thanks, a definite improvement!

@vstinner vstinner enabled auto-merge (squash) May 31, 2023 11:17
@vstinner vstinner merged commit 85e5d03 into python:main May 31, 2023
@miss-islington
Copy link
Contributor

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry, @vstinner, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 85e5d03163cac106ac8ec142ef03f1349a48948b 3.12

@miss-islington
Copy link
Contributor

Sorry @vstinner, I had trouble checking out the 3.11 backport branch.
Please retry by removing and re-adding the "needs backport to 3.11" label.
Alternatively, you can backport using cherry_picker on the command line.
cherry_picker 85e5d03163cac106ac8ec142ef03f1349a48948b 3.11

@vstinner vstinner deleted the wave_doc branch May 31, 2023 11:39
vstinner added a commit to vstinner/cpython that referenced this pull request May 31, 2023
Add ".. class::" markups in the wave documentation.

* Reformat also wave.py (minor PEP 8 changes).
* Remove redundant "import struct": it's already imported at top
  level.
* Remove wave.rst from .nitignore

(cherry picked from commit 85e5d03)
@bedevere-bot
Copy link

GH-105138 is a backport of this pull request to the 3.12 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.12 bug and security fixes label May 31, 2023
@vstinner vstinner removed the needs backport to 3.11 only security fixes label May 31, 2023
vstinner added a commit that referenced this pull request May 31, 2023
gh-105096: Reformat wave documentation (#105136)

Add ".. class::" markups in the wave documentation.

* Reformat also wave.py (minor PEP 8 changes).
* Remove redundant "import struct": it's already imported at top
  level.
* Remove wave.rst from .nitignore

(cherry picked from commit 85e5d03)
vstinner added a commit to vstinner/cpython that referenced this pull request May 31, 2023
…ython#105138)

pythongh-105096: Reformat wave documentation (python#105136)

Add ".. class::" markups in the wave documentation.

* Reformat also wave.py (minor PEP 8 changes).
* Remove redundant "import struct": it's already imported at top
  level.
* Remove wave.rst from .nitignore

(cherry picked from commit 85e5d03)
(cherry picked from commit 01b42f9)
vstinner added a commit that referenced this pull request May 31, 2023
…05155)

[3.12] gh-105096: Reformat wave documentation (#105136) (#105138)

gh-105096: Reformat wave documentation (#105136)

Add ".. class::" markups in the wave documentation.

* Reformat also wave.py (minor PEP 8 changes).
* Remove redundant "import struct": it's already imported at top
  level.
* Remove wave.rst from .nitignore

(cherry picked from commit 85e5d03)
(cherry picked from commit 01b42f9)
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.

4 participants