Skip to content

Docs: align usage of versionadded with recommended practice #114409

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

Conversation

erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented Jan 21, 2024

@erlend-aasland
Copy link
Contributor Author

@CAM-Gerlach, let me know if you want me to split this up.

@CAM-Gerlach
Copy link
Member

CAM-Gerlach commented Jan 21, 2024

For some background, a while ago I leaned in favor of versionadded for parameters for reasons of concision, and encouraged its use (including by folks like Erlend), but upon further experience and practical use I later become solidly convinced (for more compelling semantic reasons) that while the current status quo is not ideal (as there should be a structured way to indicate the addition or removal of a specific parameter 1), it is much more sensible and consistent to use versionadded for additions/removals of the current feature/object scope (module, class, method, function, etc), whereas absent a better alternative (which I and others have been working on), versionchanged is the most appropriate be used for additions, changes and (arguably) removals of callable parameters.

Comparing current usage on main, I found 293 hits in .rst files under /Doc for versionchanged::.*(\n){0,1}.*([Aa]dd.*([pP]aram|[Aa]rg)|([pP]aram|[Aa]rg).*[Aa]dd), which matches versionchanged directives that mention both "Add" and "param"/"arg" in their description, and only 76 hits for versionadded::.*(\n){0,1}.*([Pp]aram|[Aa]rg) (including 4 resolved in PR #114403 , and a number added at my own instigation, an over 4:1 ratio in all.

(To note though @erlend-aasland this PR appears to only catch those that contain param(eter), but a roughly equal number use arg(ument)? instead.)

Full results for `versionadded` with params/args in body
versionadded::.*(\n){0,1}.*([Pp]aram|[Aa]rg)" (76 hits in 30 files of 465 searched)
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\c-api\code.rst (1 hit)
	Line  66:    .. versionadded:: 3.8 as ``PyCode_NewWithPosOnlyArgs``
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\argparse.rst (1 hit)
	Line 1939:    .. versionadded:: 3.4
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\asyncio-stream.rst (1 hit)
	Line  80:    .. versionadded:: 3.8
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\bdb.rst (1 hit)
	Line 135:    .. versionadded:: 3.1
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\concurrent.futures.rst (1 hit)
	Line 174:    .. versionadded:: 3.6
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\configparser.rst (4 hits)
	Line 1048:       .. versionadded:: 3.2
	Line 1052:       .. versionadded:: 3.6.1
	Line 1055:       .. versionadded:: 3.7
	Line 1294:    .. versionadded:: 3.2
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\datetime.rst (5 hits)
	Line  862:    .. versionadded:: 3.6
	Line 1261:    .. versionadded:: 3.6
	Line 1505:    .. versionadded:: 3.6
	Line 1840:    .. versionadded:: 3.6
	Line 1884:    .. versionadded:: 3.6
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\difflib.rst (2 hits)
	Line  55:    .. versionadded:: 3.2
	Line 386:    .. versionadded:: 3.2
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\email.policy.rst (1 hit)
	Line 221:       .. versionadded:: 3.5
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\functions.rst (2 hits)
	Line 1076:    .. versionadded:: 3.4
	Line 1114:    .. versionadded:: 3.4
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\functools.rst (1 hit)
	Line 278:    .. versionadded:: 3.9
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\http.client.rst (1 hit)
	Line 464:    .. versionadded:: 3.6
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\http.server.rst (5 hits)
	Line 331:    .. versionadded:: 3.7
	Line 441: .. versionadded:: 3.4
	Line 444: .. versionadded:: 3.8
	Line 453: .. versionadded:: 3.7
	Line 462: .. versionadded:: 3.11
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\logging.config.rst (1 hit)
	Line 130:     .. versionadded:: 3.10
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\logging.handlers.rst (1 hit)
	Line  874:    .. versionadded:: 3.3
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\logging.rst (3 hits)
	Line  618:    .. versionadded:: 3.2
	Line  621:    .. versionadded:: 3.8
	Line  624:    .. versionadded:: 3.10
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\multiprocessing.shared_memory.rst (1 hit)
	Line  91:    .. versionadded:: 3.13
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\os.rst (14 hits)
	Line 1254:    .. versionadded:: 3.3
	Line 2023:    .. versionadded:: 3.3
	Line 2199:    .. versionadded:: 3.3
	Line 2364:    .. versionadded:: 3.3
	Line 2398:    .. versionadded:: 3.2
	Line 2432:    .. versionadded:: 3.3
	Line 2454:    .. versionadded:: 3.3
	Line 2535:    .. versionadded:: 3.3
	Line 2566:    .. versionadded:: 3.3
	Line 2617:    .. versionadded:: 3.3
	Line 2673:    .. versionadded:: 3.3
	Line 2962:    .. versionadded:: 3.3
	Line 3383:    .. versionadded:: 3.3
	Line 3432:    .. versionadded:: 3.3
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\pdb.rst (2 hits)
	Line 212:    .. versionadded:: 3.1
	Line 215:    .. versionadded:: 3.2
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\pickletools.rst (1 hit)
	Line  97:    .. versionadded:: 3.2
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\shutil.rst (1 hit)
	Line 292:    .. versionadded:: 3.8
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\sqlite3.rst (4 hits)
	Line  346:    .. versionadded:: 3.4
	Line  355:    .. versionadded:: 3.12
	Line  750:       .. versionadded:: 3.8
	Line 1135:       .. versionadded:: 3.12
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\subprocess.rst (5 hits)
	Line  311:    .. versionadded:: 3.6
	Line  314:    .. versionadded:: 3.7
	Line  687:    .. versionadded:: 3.10
	Line 1541:    .. versionadded:: 3.11
	Line 1559:    .. versionadded:: 3.11
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\unittest.rst (1 hit)
	Line 2199:    .. versionadded:: 3.12
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\urllib.parse.rst (1 hit)
	Line 732:    .. versionadded:: 3.5
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\venv.rst (3 hits)
	Line 204:     .. versionadded:: 3.6
	Line 207:     .. versionadded:: 3.9
	Line 210:     .. versionadded:: 3.13
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\xml.etree.elementtree.rst (6 hits)
	Line  708:    .. versionadded:: 3.4
	Line  711:    .. versionadded:: 3.8
	Line  735:    .. versionadded:: 3.4
	Line  738:    .. versionadded:: 3.8
	Line  861:    .. versionadded:: 3.9
	Line 1192:       .. versionadded:: 3.4
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\xml.sax.utils.rst (1 hit)
	Line 74:    .. versionadded:: 3.2
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\zipapp.rst (1 hit)
	Line 174:    .. versionadded:: 3.7
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\zipfile.rst (4 hits)
	Line 244:    .. versionadded:: 3.8
	Line 651:    .. versionadded:: 3.2
	Line 707:       .. versionadded:: 3.4
	Line 752:   .. versionadded:: 3.8

This also follows the established guidance in the devguide:

versionadded

This directive documents the version of Python which added the described feature, or a part of it, to the library or C API.

versionchanged

Similar to versionadded, but describes when and what changed in the named feature in some way (new parameters, changed side effects, platform support, etc.).

Footnotes

  1. Ideally we should have a more concise and structured way to indicate that a parameter was added/changed/deprecated/removed, and I have a prototype in the works as part of reStructured Data, but it's not a super high priority at the moment given we currently don't use a structured format for parameter information to begin with (outside of the parameter name in the signature). It's much more important to ensure that versionadded is used consistently to indicate that the current semantic scope was added, so what we have is reliable and we can add finer-grained structure later as desired.

@CAM-Gerlach
Copy link
Member

CAM-Gerlach commented Jan 21, 2024

As to this PR, while it's broad-reach certainly isn't ideal and it would be nice to find a better way to split it up along logical boundaries, pragmatically speaking its probably a good deal less noisy than, lots of e.g. individual PRs for every module to change a few lines. Perhaps the best we can do is split up mentions of parameters with those of anything else, and perhaps even "parameters" and "arguments" (although the distinction is more or less just spelling). But maybe best to keep as one atomic PR?

Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

To note, this mostly appears to address instances where param(eter)? (and perhaps "attribute") was mentioned in a versionadded, but not arg(ument)?, which a roughly equal number of instances use to refer to the same thing.

Full results for `versionadded` with params/args in body
versionadded::.*(\n){0,1}.*([Pp]aram|[Aa]rg)" (76 hits in 30 files of 465 searched)
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\c-api\code.rst (1 hit)
	Line  66:    .. versionadded:: 3.8 as ``PyCode_NewWithPosOnlyArgs``
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\argparse.rst (1 hit)
	Line 1939:    .. versionadded:: 3.4
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\asyncio-stream.rst (1 hit)
	Line  80:    .. versionadded:: 3.8
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\bdb.rst (1 hit)
	Line 135:    .. versionadded:: 3.1
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\concurrent.futures.rst (1 hit)
	Line 174:    .. versionadded:: 3.6
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\configparser.rst (4 hits)
	Line 1048:       .. versionadded:: 3.2
	Line 1052:       .. versionadded:: 3.6.1
	Line 1055:       .. versionadded:: 3.7
	Line 1294:    .. versionadded:: 3.2
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\datetime.rst (5 hits)
	Line  862:    .. versionadded:: 3.6
	Line 1261:    .. versionadded:: 3.6
	Line 1505:    .. versionadded:: 3.6
	Line 1840:    .. versionadded:: 3.6
	Line 1884:    .. versionadded:: 3.6
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\difflib.rst (2 hits)
	Line  55:    .. versionadded:: 3.2
	Line 386:    .. versionadded:: 3.2
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\email.policy.rst (1 hit)
	Line 221:       .. versionadded:: 3.5
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\functions.rst (2 hits)
	Line 1076:    .. versionadded:: 3.4
	Line 1114:    .. versionadded:: 3.4
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\functools.rst (1 hit)
	Line 278:    .. versionadded:: 3.9
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\http.client.rst (1 hit)
	Line 464:    .. versionadded:: 3.6
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\http.server.rst (5 hits)
	Line 331:    .. versionadded:: 3.7
	Line 441: .. versionadded:: 3.4
	Line 444: .. versionadded:: 3.8
	Line 453: .. versionadded:: 3.7
	Line 462: .. versionadded:: 3.11
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\logging.config.rst (1 hit)
	Line 130:     .. versionadded:: 3.10
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\logging.handlers.rst (1 hit)
	Line  874:    .. versionadded:: 3.3
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\logging.rst (3 hits)
	Line  618:    .. versionadded:: 3.2
	Line  621:    .. versionadded:: 3.8
	Line  624:    .. versionadded:: 3.10
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\multiprocessing.shared_memory.rst (1 hit)
	Line  91:    .. versionadded:: 3.13
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\os.rst (14 hits)
	Line 1254:    .. versionadded:: 3.3
	Line 2023:    .. versionadded:: 3.3
	Line 2199:    .. versionadded:: 3.3
	Line 2364:    .. versionadded:: 3.3
	Line 2398:    .. versionadded:: 3.2
	Line 2432:    .. versionadded:: 3.3
	Line 2454:    .. versionadded:: 3.3
	Line 2535:    .. versionadded:: 3.3
	Line 2566:    .. versionadded:: 3.3
	Line 2617:    .. versionadded:: 3.3
	Line 2673:    .. versionadded:: 3.3
	Line 2962:    .. versionadded:: 3.3
	Line 3383:    .. versionadded:: 3.3
	Line 3432:    .. versionadded:: 3.3
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\pdb.rst (2 hits)
	Line 212:    .. versionadded:: 3.1
	Line 215:    .. versionadded:: 3.2
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\pickletools.rst (1 hit)
	Line  97:    .. versionadded:: 3.2
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\shutil.rst (1 hit)
	Line 292:    .. versionadded:: 3.8
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\sqlite3.rst (4 hits)
	Line  346:    .. versionadded:: 3.4
	Line  355:    .. versionadded:: 3.12
	Line  750:       .. versionadded:: 3.8
	Line 1135:       .. versionadded:: 3.12
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\subprocess.rst (5 hits)
	Line  311:    .. versionadded:: 3.6
	Line  314:    .. versionadded:: 3.7
	Line  687:    .. versionadded:: 3.10
	Line 1541:    .. versionadded:: 3.11
	Line 1559:    .. versionadded:: 3.11
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\unittest.rst (1 hit)
	Line 2199:    .. versionadded:: 3.12
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\urllib.parse.rst (1 hit)
	Line 732:    .. versionadded:: 3.5
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\venv.rst (3 hits)
	Line 204:     .. versionadded:: 3.6
	Line 207:     .. versionadded:: 3.9
	Line 210:     .. versionadded:: 3.13
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\xml.etree.elementtree.rst (6 hits)
	Line  708:    .. versionadded:: 3.4
	Line  711:    .. versionadded:: 3.8
	Line  735:    .. versionadded:: 3.4
	Line  738:    .. versionadded:: 3.8
	Line  861:    .. versionadded:: 3.9
	Line 1192:       .. versionadded:: 3.4
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\xml.sax.utils.rst (1 hit)
	Line 74:    .. versionadded:: 3.2
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\zipapp.rst (1 hit)
	Line 174:    .. versionadded:: 3.7
  C:\Users\C. A. M. Gerlach\Documents\dev\Python\cpython\Doc\library\zipfile.rst (4 hits)
	Line 244:    .. versionadded:: 3.8
	Line 651:    .. versionadded:: 3.2
	Line 707:       .. versionadded:: 3.4
	Line 752:   .. versionadded:: 3.8

.. versionadded:: 3.9
Added the function :func:`cache_parameters`
.. versionchanged:: 3.9
Added the function :func:`!cache_parameters`
Copy link
Member

Choose a reason for hiding this comment

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

It sounds to me based on this writing that this function was added to the module (which, as far as I can tell, it wasn't), as opposed to being an additional behavior in cmp_to_key. Assuming the latter is in fact the case, and since its non-trivial to find more information about it (not being crosslinked), I'd rewrite this as something like:

Suggested change
Added the function :func:`!cache_parameters`
Instrument the wrapped callable with a :func:`!cache_parameters` function
to retrieve the *maxsize* and *typed* of its cache.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, but I deliberately did not change it. It may be beneficial to address this in a separate PR.

@erlend-aasland
Copy link
Contributor Author

To note, this mostly appears to address instances where param(eter)? (and perhaps "attribute") was mentioned in a versionadded, but not arg(ument)?, which a roughly equal number of instances use to refer to the same thing.

Yes, I grepped for instances of param*. arg* should be included. I also fixed some spurious attributes, but I did not deliberately search for those.

But maybe best to keep as one atomic PR?

For this type of change, I think it is best to do it all in one atomic PR; IMO, it is not worth the CI churn to split it up. We can land this pretty quick.

@erlend-aasland
Copy link
Contributor Author

Thanks for your thorough reviews, CAM and Ezio! 🙏

@erlend-aasland erlend-aasland enabled auto-merge (squash) January 22, 2024 21:17
@CAM-Gerlach
Copy link
Member

Mmm, as I feared...zipfile is already off the exclude list, so we either have to fix it here, or just make sure we remember it for a followup. As much as part of me would prefer the former, realistically the latter is presumably the better approach.

@erlend-aasland
Copy link
Contributor Author

Mmm, as I feared...zipfile is already off the exclude list, so we either have to fix it here, or just make sure we remember it for a followup. As much as part of me would prefer the former, realistically the latter is presumably the better approach.

Yeah, I went for that!

Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

LGTM now, thanks @erlend-aasland !

@CAM-Gerlach
Copy link
Member

How come all the tests are kicking off, instead of just the docs-relevant ones? It only contains changes to rst files under Doc/, so the other tests shouldn't run...did we change something with that lately?

@erlend-aasland erlend-aasland merged commit 1d7bddd into python:main Jan 22, 2024
@erlend-aasland erlend-aasland deleted the docs/align-versionchanged-with-recommended-practise branch January 22, 2024 21:40
@miss-islington-app

This comment was marked as outdated.

@miss-islington-app

This comment was marked as outdated.

@miss-islington-app

This comment was marked as outdated.

@CAM-Gerlach
Copy link
Member

Unsurprisingly, the backport failed due to conflicts on params, etc. added in 3.13...maybe I should have suggested pulling those out to a separate one, oops. Like a hand with the backport, or you got it already?

@erlend-aasland
Copy link
Contributor Author

Unsurprisingly, the backport failed due to conflicts on params, etc. added in 3.13...maybe I should have suggested pulling those out to a separate one, oops. Like a hand with the backport, or you got it already?

I can do the backports tomorrow (it's getting late over here). Feel free to take care of them, if you feel the urge.

@bedevere-app
Copy link

bedevere-app bot commented Jan 23, 2024

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

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Jan 23, 2024
erlend-aasland added a commit to erlend-aasland/cpython that referenced this pull request Jan 23, 2024
…ded practice (python#114409)

(cherry picked from commit 1d7bddd)

Co-authored-by: C.A.M. Gerlach <[email protected]>
Co-authored-by: Ezio Melotti <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Jan 23, 2024

GH-114473 is a backport of this pull request to the 3.11 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.11 only security fixes label Jan 23, 2024
@erlend-aasland
Copy link
Contributor Author

@CAM-Gerlach, I did the backports. I got the feeling we might have missed some occurrences in os.rst.

erlend-aasland added a commit that referenced this pull request Jan 23, 2024
…ded practice (#114409) (#114473)

(cherry picked from commit 1d7bddd)

Co-authored-by: C.A.M. Gerlach <[email protected]>
Co-authored-by: Ezio Melotti <[email protected]>
erlend-aasland added a commit that referenced this pull request Jan 23, 2024
…ded practice (#114409) (#114472)

(cherry picked from commit 1d7bddd)

Co-authored-by: C.A.M. Gerlach <[email protected]>
Co-authored-by: Ezio Melotti <[email protected]>
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip issue skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants