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

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.

@github-actions

Copy link
Copy Markdown

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that's incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.

@github-actions github-actions Bot added the stale label Sep 26, 2026
@takayoshi-makabe
takayoshi-makabe force-pushed the fix-conversions-fallback-messages branch from eefd5b4 to 528e3a7 Compare September 27, 2026 14:09
@github-actions github-actions Bot removed the stale label Sep 28, 2026

This branch has not been deployed

No deployments
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