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

bpo-47220: Document the optional callback parameter of weakref.WeakMethod by geryogam · Pull Request #25491 · python/cpython · GitHub

/ cpython Public

bpo-47220: Document the optional callback parameter of weakref.WeakMethod - #25491

Merged
rhettinger merged 14 commits into
python:mainfrom
geryogam:patch-25
Nov 30, 2022
Merged

bpo-47220: Document the optional callback parameter of weakref.WeakMethod#25491
rhettinger merged 14 commits into
python:mainfrom
geryogam:patch-25

Conversation

geryogam commented Apr 20, 2021
edited
Loading

Copy link
Copy Markdown
Contributor

This PR will bring the following changes to the weakref documentation:

  • document the optional callback parameter of weakref.WeakMethod;
  • normalise the use of ‘proxy’, replacing the mix of ‘proxy’ (8 occurrences) and ‘proxy object’ (3 occurrences);
  • normalise the use of ‘weak reference’, replacing the mix of ‘weak reference’ (37 occurrences), ‘weak reference object’ (7 occurrences), ‘reference’ (7 occurrences), ‘reference object’ (6 occurrences), ‘weakref’ (3 occurrences), and ‘weakref object’ (1 occurrence).

https://bugs.python.org/issue47220

geryogam marked this pull request as ready for review April 23, 2021 22:03

github-actions Bot commented Jun 3, 2021

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 Jun 3, 2021
JelleZijlstra self-requested a review April 2, 2022 23:20

Copy link
Copy Markdown
Member

Sorry this had to wait for so long!

  • Documenting the callback parameter to WeakMethod requires an issue and a NEWS entry in my view. Also, the documentation should say what the parameter does.
  • Some of the wording changes feel gratuitous (e.g. "proxy objects" vs. "proxies"). I don't know if the original authors had reasons for using one form or the other, so I'd be hesitant to make changes without a strong reason.

JelleZijlstra removed their request for review April 3, 2022 03:30
Comment thread Doc/library/weakref.rst Outdated
geryogam changed the title Update weakref.rst bpo-47220: Document the optional callback parameter of weakref.WeakMethod Apr 4, 2022
geryogam changed the title bpo-47220: Document the optional callback parameter of weakref.WeakMethod Document the optional callback parameter of weakref.WeakMethod Apr 4, 2022
geryogam changed the title Document the optional callback parameter of weakref.WeakMethod bpo-47220: Document the optional callback parameter of weakref.WeakMethod Apr 4, 2022

geryogam commented Apr 4, 2022
edited
Loading

Copy link
Copy Markdown
Contributor Author

@JelleZijlstra @merwok Thanks for the review.

  • Documenting the callback parameter to WeakMethod requires an issue and a NEWS entry in my view. Also, the documentation should say what the parameter does.

Done.

  • Some of the wording changes feel gratuitous (e.g. "proxy objects" vs. "proxies"). I don't know if the original authors had reasons for using one form or the other, so I'd be hesitant to make changes without a strong reason.

I normalised the use of ‘proxy’ because a mix of ‘proxy’ (8 occurrences) and ‘proxy object’ (3 occurrences) was used.

Copy link
Copy Markdown

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.

Copy link
Copy Markdown
Contributor Author

Please remove all unrelated changes.

Reverted. I have made the requested changes; please review again.

Copy link
Copy Markdown

Thanks for making the requested changes!

@erlend-aasland: please review the changes made to this pull request.

rhettinger removed the needs backport to 3.10 only security fixes label Nov 30, 2022
rhettinger merged commit 9628136 into python:main Nov 30, 2022

Copy link
Copy Markdown
Contributor

Thanks @maggyero for the PR, and @rhettinger for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

Copy link
Copy Markdown
Contributor

Sorry @maggyero and @rhettinger, 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 9628136fac997847b4662e6a17faf06d2a0507eb 3.11

rhettinger added needs backport to 3.11 only security fixes and removed needs backport to 3.11 only security fixes labels Nov 30, 2022

Copy link
Copy Markdown
Contributor

Thanks @maggyero for the PR, and @rhettinger for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

Copy link
Copy Markdown

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

bedevere-bot removed the needs backport to 3.11 only security fixes label Nov 30, 2022
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 30, 2022
…thod (pythonGH-25491)

(cherry picked from commit 9628136)

Co-authored-by: Géry Ogam <gery.ogam@gmail.com>
miss-islington added a commit that referenced this pull request Nov 30, 2022
…thod (GH-25491)

(cherry picked from commit 9628136)

Co-authored-by: Géry Ogam <gery.ogam@gmail.com>

Copy link
Copy Markdown
Contributor Author

Thanks for the review!

geryogam deleted the patch-25 branch November 30, 2022 20:00
carljm added a commit to carljm/cpython that referenced this pull request Dec 1, 2022
* main: (112 commits)
  pythongh-99894: Ensure the local names don't collide with the test file in traceback suggestion error checking (python#99895)
  pythongh-99612: Fix PyUnicode_DecodeUTF8Stateful() for ASCII-only data (pythonGH-99613)
  Doc: Add summary line to isolation_level & autocommit sqlite3.connect params (python#99917)
  pythonGH-98906 ```re``` module: ```search() vs. match()``` section should mention ```fullmatch()``` (pythonGH-98916)
  pythongh-89189: More compact range iterator (pythonGH-27986)
  bpo-47220: Document the optional callback parameter of weakref.WeakMethod (pythonGH-25491)
  pythonGH-99905: Fix output of misses in summarize_stats.py execution counts (pythonGH-99906)
  pythongh-99845: PEP 670: Convert PyObject macros to functions (python#99850)
  pythongh-99845: Use size_t type in __sizeof__() methods (python#99846)
  pythonGH-99877)
  Fix typo in exception message in `multiprocessing.pool` (python#99900)
  pythongh-87092: move all localsplus preparation into separate function called from assembler stage (pythonGH-99869)
  pythongh-99891: Fix infinite recursion in the tokenizer when showing warnings (pythonGH-99893)
  pythongh-99824: Document that sqlite3.connect implicitly open a transaction if autocommit=False (python#99825)
  pythonGH-81057: remove static state from suggestions.c (python#99411)
  Improve zip64 limit error message (python#95892)
  pythongh-98253: Break potential reference cycles in external code worsened by typing.py lru_cache (python#98591)
  pythongh-99127: Allow some features of syslog to the main interpreter only (pythongh-99128)
  pythongh-82836: fix private network check (python#97733)
  Docs: improve accuracy of socketserver reference (python#24767)
  ...
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
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.


Back | FazBrowse Home | New Git URL