Skip to content

gh-141510: Update PyDict C API doc for frozendict - #145533

Merged
vstinner merged 6 commits into
python:mainfrom
vstinner:frozendict_capi_doc
Mar 12, 2026
Merged

gh-141510: Update PyDict C API doc for frozendict#145533
vstinner merged 6 commits into
python:mainfrom
vstinner:frozendict_capi_doc

Conversation

@vstinner

@vstinnervstinner commented Mar 5, 2026•
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Member

@bedevere-appbedevere-appBot added the docs Documentation in the Doc dir label Mar 5, 2026
@bedevere-appbedevere-appBot mentioned this pull request Mar 5, 2026
Closed
@vstinner

vstinner commented Mar 5, 2026

Copy link
Copy Markdown
MemberAuthor

I used test_capi.test_dict to check if PyDict functions accept frozendict or not.

cc @encukou

encukou
encukou reviewed Mar 5, 2026
View reviewed changes

@encukouencukou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You've missed PyDict_Contains.

I think the info for the current behaviour should be in the main text, not in the versionchanged directive. If nothing else, there's been a suggestion to hide these for older versions.

Comment threadDoc/c-api/dict.rst Outdated
@vstinner

vstinner commented Mar 5, 2026

Copy link
Copy Markdown
MemberAuthor

You've missed PyDict_Contains.

Oops, fixed.

I think the info for the current behaviour should be in the main text, not in the versionchanged directive.

I updated the PR to mention frozendict in the main text. I kept versionchanged. I'm not sure if you want both, or only changing the main text.

@vstinner

vstinner commented Mar 9, 2026

Copy link
Copy Markdown
MemberAuthor

@encukou and/or @ZeroIntensity: Do you want to review this change?

ZeroIntensity
ZeroIntensity reviewed Mar 9, 2026
View reviewed changes
Comment threadDoc/c-api/dict.rst Outdated
Comment threadDoc/c-api/dict.rst Outdated
encukou
encukou reviewed Mar 10, 2026
View reviewed changes
Comment threadDoc/c-api/dict.rst
Comment on lines +48 to +50
.. versionchanged:: next
Also accept :class:`frozendict`.

@encukouencukouMar 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Now, do we need versionchanged at all, since frozendict was added in this version?

Suggested change
.. versionchanged:: next
Also accept :class:`frozendict`.

@ZeroIntensityZeroIntensityMar 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it's nice to have an explicit historical record.

@vstinnervstinnerMar 10, 2026

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I prefer to be explicit about the change.

ZeroIntensity
ZeroIntensity approved these changes Mar 10, 2026
View reviewed changes

@ZeroIntensityZeroIntensity left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@vstinner
vstinner merged commit d4cc553 into python:mainMar 12, 2026
32 checks passed
@vstinner
vstinner deleted the frozendict_capi_doc branch March 12, 2026 14:04
@github-project-automationgithub-project-automationBot moved this from Todo to Done in Docs PRsMar 12, 2026
@vstinner

vstinner commented Mar 12, 2026

Copy link
Copy Markdown
MemberAuthor

PR merged, thanks for your reviews!

ljfp pushed a commit to ljfp/cpython that referenced this pull request Apr 25, 2026
@vstinner@ljfp
pythongh-141510: Update PyDict C API doc for frozendict (python#145533)
00ed59c
Mention frozendict support.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@encukouencukouencukou left review comments
@ZeroIntensityZeroIntensityZeroIntensity approved these changes

Assignees

No one assigned

Labels

docsDocumentation in the Doc dirskip news

Projects

Docs PRs
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@vstinner@encukou@ZeroIntensity