fix: avoid using HasField on scalar primitives in DataConverter#985
Merged
patricklundquist merged 3 commits intomasterfrom Mar 13, 2026
Merged
Conversation
In proto3, standard scalar fields like string_value or int_value do not support `HasField()`. Calling `HasField()` on these fields raises a ValueError. This commit updates `clarifai/runners/utils/data_utils.py` to evaluate the truthiness or explicit default values of these scalar fields instead of using `HasField()`. `HasField()` calls were retained when checking `pb_value` in `Param.from_proto` because `pb_value` is `google.protobuf.Value` and these variants are part of a oneof. Co-authored-by: patricklundquist <1460278+patricklundquist@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the runner-side DataConverter utilities to avoid calling HasField() on proto3 scalar primitives (which raises ValueError), while retaining HasField() usage where fields are part of a oneof (google.protobuf.Value).
Changes:
- Remove
HasField()checks for scalar primitive fields (e.g.,string_value) and switch to value/default comparisons. - Update
is_old_format()to detect “old format” using scalar default checks instead ofHasField()for those scalars. - Remove redundant in-function
jsonimports (module-levelimport jsonis used).
Contributor
Author
|
@copilot open a new pull request to apply changes based on the comments in this thread |
Contributor
|
@patricklundquist I've opened a new pull request, #986, to work on those changes. Once the pull request is ready, I'll request review from you. |
Added JSON import to handle default values in methods.
…it tests (#986) * Initial plan * refactor: extract _has_scalar_set helper and add is_old_format tests Co-authored-by: patricklundquist <1460278+patricklundquist@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: patricklundquist <1460278+patricklundquist@users.noreply.github.com>
Minimum allowed line rate is |
ackizilkale
approved these changes
Mar 13, 2026
patricklundquist
deleted the
fix-data-scalar-fields-hasfield-9151022936464392563
branch
Closed
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.
In proto3, standard scalar fields like string_value or int_value do not support
HasField(). CallingHasField()on these fields raises a ValueError. This commit updatesclarifai/runners/utils/data_utils.pyto evaluate the truthiness or explicit default values of these scalar fields instead of usingHasField().HasField()calls were retained when checkingpb_valueinParam.from_protobecausepb_valueisgoogle.protobuf.Valueand these variants are part of a oneof.