Honor converter hooks in make_dict_unstructure_fn - #783
Open
Vinayak19112003 wants to merge 1 commit into
Open
Vinayak19112003 wants to merge 1 commit into
Vinayak19112003 wants to merge 1 commit into
Conversation
make_dict_unstructure_fn ignored hooks registered on the converter (for example methods picked up by the use_class_methods strategy), so hook factories built on top of it could not compose with those customizations. When no customizations are requested, prefer the converter's own unstructure hook for the class over generating a new function. Factories already being evaluated for the class are skipped so lower-precedence factories get a chance; in-progress factory invocations are now tracked (thread-local) in FunctionDispatch.dispatch for this. Fixes python-attrs#566.
This branch has not been deployed
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
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #566.
What was wrong:
make_dict_unstructure_fnalways generated a fresh attribute-based unstructure function, ignoring hooks already registered on the converter. So a hook factory built on top of it (like the_type-tagging factory in #566) could not be combined with theuse_class_methodsstrategy — the metamethod was silently dropped.The fix: when no customizations are requested (no attribute overrides, no
omit_if_default/include_init_false/explicit alias setting),make_dict_unstructure_fnfirst asks the converter for its own unstructure hook for the class and prefers it over generating one. That covers methods picked up byuse_class_methodsas well as hooks registered directly for the class. Factories already being evaluated for the class are skipped so lower-precedence ones get a chance instead of recursing;FunctionDispatch.dispatchnow tracks in-progress factory invocations (thread-local, observational only, no change to dispatch behavior itself) to detect that.How I tested:
make_unstructure_dict_unstructure_fndoes not honoruse_class_methods#566 on current main: before the fix the second converter produced{'a': 1, '_type': 'Thing'}(metamethod ignored); after the fix it produces{'a': '1', '_type': 'Thing'}.test_make_dict_unstructure_fn_honors_class_methodsintests/strategies/test_class_methods.py(fails on main, passes with the fix).bson/pyyamlextras couldn't be collected in this environment; unrelated).ruff checkandruff format --checkclean on all touched files.