Skip to content

Fix the cache_result parameter name in get_*_hook docstrings - #815

Merged
Tinche merged 1 commit into
python-attrs:mainfrom
mmporong:fix-hook-cache-result-docs
Oct 28, 2026
Merged

Tinche merged 1 commit into
python-attrs:mainfrom
mmporong:fix-hook-cache-result-docs

Conversation

@mmporong

Copy link
Copy Markdown
Contributor

BaseConverter.get_unstructure_hook() and BaseConverter.get_structure_hook() document their second argument as :param cache:, but the parameter is cache_result (since 24.1.0), so the API docs list a keyword that does not exist:

>>> cattrs.Converter().get_unstructure_hook(int, cache=False)
TypeError: BaseConverter.get_unstructure_hook() got an unexpected keyword argument 'cache'
>>> cattrs.Converter().get_unstructure_hook(int, cache_result=False)
<function identity at ...>

Same for get_structure_hook(). Converter does not override either method, so the base docstrings are the ones shown for both converters.

This renames the two :param fields to cache_result. Docstring-only, so I did not add a HISTORY.md entry; happy to add one if you want it listed.

@codspeed

codspeed Bot commented Oct 28, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

1 note

ℹ️ 64 benchmarks have no flamegraph

The flamegraph wasn't generated: the symbols of the profiling trace could not be resolved, because no information about the language frames was collected.

See affected benchmarks

✅ 64 untouched benchmarks


Comparing mmporong:fix-hook-cache-result-docs (5310648) with main (bde101a)1

Open in CodSpeed

Footnotes

  1. No successful run was found on main (7a24f77) during the generation of this report, so bde101a was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

@Tinche

Tinche commented Oct 28, 2026

Copy link
Copy Markdown
Member

Great, thanks!

@Tinche
Tinche merged commit d043dbd into python-attrs:main Oct 28, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants