Skip to content

Fix misleading error messages in conversions dispatch fallbacks - #3856

Open
takayoshi-makabe wants to merge 1 commit into
apache:mainfrom
takayoshi-makabe:fix-conversions-fallback-messages
Open

Fix misleading error messages in conversions dispatch fallbacks#3856
takayoshi-makabe wants to merge 1 commit into
apache:mainfrom
takayoshi-makabe:fix-conversions-fallback-messages

Conversation

@takayoshi-makabe

Copy link
Copy Markdown

Rationale for this change

The singledispatch fallbacks in conversions.py describe the wrong operation, so an unregistered type produces a message that sends you looking in the wrong place. Passing a type with no registered implementation to each of the five entry points today:

partition_to_py  Cannot convert 'foo' to unsupported type: FooUnknownType()
to_bytes         scale does not match FooUnknownType()
from_bytes       Cannot deserialize bytes, type FooUnknownType() not supported: b'foo'
to_json          Cannot deserialize bytes, type FooUnknownType() not supported: foo
from_json        Cannot deserialize bytes, type FooUnknownType() not supported: foo

This reworks the three to follow the shape from_bytes already uses, so each names the operation it performs and the value it rejected:

to_bytes         Cannot serialize to bytes, type FooUnknownType() not supported: b'foo'
to_json          Cannot serialize to JSON, type FooUnknownType() not supported: foo
from_json        Cannot deserialize JSON, type FooUnknownType() not supported: foo

partition_to_py and from_bytes are already accurate and are left alone.

One small change beyond the strings: to_bytes takes its value parameter as value instead of _ so the message can include it, matching the other four. It is rendered with !r, like from_bytes, because the parameter also accepts bytes.

Are these changes tested?

Yes. test_raise_on_unregistered_type asserted the old to_bytes wording, so it is updated. to_json and from_json had no fallback coverage at all — which is how the copied text survived — so cases for both are added and all five entry points are now covered.

Are there any user-facing changes?

Only the text of these TypeError messages. No behaviour changes: the same inputs raise the same exception type in the same places.

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.

1 participant