-
Notifications
You must be signed in to change notification settings - Fork 37
fix: remove float_precision for protobuf 7 #559
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
bcd7ecd
a27845c
d946a7a
89e5ac2
41c8499
3b4a2e0
638c8f6
e6ef3c7
944c570
586cb92
9a24832
6672f08
e76ad73
ae854bf
e14707c
18e10e8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -36,6 +36,9 @@ | |||||
|
|
||||||
| PROTOBUF_VERSION = google.protobuf.__version__ | ||||||
|
|
||||||
| # extract the major version code | ||||||
| _PROTOBUF_MAJOR_VERSION = PROTOBUF_VERSION.partition(".")[0] | ||||||
|
|
||||||
| _upb = has_upb() # Important to cache result here. | ||||||
|
|
||||||
|
|
||||||
|
|
@@ -383,7 +386,7 @@ def _warn_if_including_default_value_fields_is_used_protobuf_5( | |||||
| including_default_value_fields (Optional(bool)): The value of `including_default_value_fields` set by the user. | ||||||
| """ | ||||||
| if ( | ||||||
| PROTOBUF_VERSION[0] not in ("3", "4") | ||||||
| _PROTOBUF_MAJOR_VERSION not in ("3", "4") | ||||||
| and including_default_value_fields is not None | ||||||
| ): | ||||||
| warnings.warn( | ||||||
|
|
@@ -491,7 +494,7 @@ def to_json( | |||||
| An indent level of 0 or negative will only insert newlines. | ||||||
| Pass None for the most compact representation without newlines. | ||||||
| float_precision (Optional(int)): If set, use this to specify float field valid digits. | ||||||
| Default is None. | ||||||
| Default is None. [DEPRECATED] float_precision was removed in Protobuf 7.x. | ||||||
| always_print_fields_with_no_presence (Optional(bool)): If True, fields without | ||||||
| presence (implicit presence scalars, repeated fields, and map fields) will | ||||||
| always be serialized. Any field that supports presence is not affected by | ||||||
|
|
@@ -501,36 +504,7 @@ def to_json( | |||||
| Returns: | ||||||
| str: The json string representation of the protocol buffer. | ||||||
| """ | ||||||
|
|
||||||
| print_fields = cls._normalize_print_fields_without_presence( | ||||||
| always_print_fields_with_no_presence, including_default_value_fields | ||||||
| ) | ||||||
|
|
||||||
| if PROTOBUF_VERSION[0] in ("3", "4"): | ||||||
| return MessageToJson( | ||||||
| cls.pb(instance), | ||||||
| use_integers_for_enums=use_integers_for_enums, | ||||||
| including_default_value_fields=print_fields, | ||||||
| preserving_proto_field_name=preserving_proto_field_name, | ||||||
| sort_keys=sort_keys, | ||||||
| indent=indent, | ||||||
| float_precision=float_precision, | ||||||
| ) | ||||||
| else: | ||||||
| # The `including_default_value_fields` argument was removed from protobuf 5.x | ||||||
| # and replaced with `always_print_fields_with_no_presence` which very similar but has | ||||||
| # handles optional fields consistently by not affecting them. | ||||||
| # The old flag accidentally had inconsistent behavior between proto2 | ||||||
| # optional and proto3 optional fields. | ||||||
| return MessageToJson( | ||||||
| cls.pb(instance), | ||||||
| use_integers_for_enums=use_integers_for_enums, | ||||||
| always_print_fields_with_no_presence=print_fields, | ||||||
| preserving_proto_field_name=preserving_proto_field_name, | ||||||
| sort_keys=sort_keys, | ||||||
| indent=indent, | ||||||
| float_precision=float_precision, | ||||||
| ) | ||||||
| return _message_to_map(map_fn=MessageToJson, **locals()) | ||||||
|
|
||||||
| def from_json(cls, payload, *, ignore_unknown_fields=False) -> "Message": | ||||||
| """Given a json string representing an instance, | ||||||
|
|
@@ -576,7 +550,7 @@ def to_dict( | |||||
| This value must match `always_print_fields_with_no_presence`, | ||||||
| if both arguments are explicitly set. | ||||||
| float_precision (Optional(int)): If set, use this to specify float field valid digits. | ||||||
| Default is None. | ||||||
| Default is None. [DEPRECATED] float_precision was removed in Protobuf 7.x. | ||||||
| always_print_fields_with_no_presence (Optional(bool)): If True, fields without | ||||||
| presence (implicit presence scalars, repeated fields, and map fields) will | ||||||
| always be serialized. Any field that supports presence is not affected by | ||||||
|
|
@@ -588,32 +562,7 @@ def to_dict( | |||||
| Messages and map fields are represented as dicts, | ||||||
| repeated fields are represented as lists. | ||||||
| """ | ||||||
|
|
||||||
| print_fields = cls._normalize_print_fields_without_presence( | ||||||
| always_print_fields_with_no_presence, including_default_value_fields | ||||||
| ) | ||||||
|
|
||||||
| if PROTOBUF_VERSION[0] in ("3", "4"): | ||||||
| return MessageToDict( | ||||||
| cls.pb(instance), | ||||||
| including_default_value_fields=print_fields, | ||||||
| preserving_proto_field_name=preserving_proto_field_name, | ||||||
| use_integers_for_enums=use_integers_for_enums, | ||||||
| float_precision=float_precision, | ||||||
| ) | ||||||
| else: | ||||||
| # The `including_default_value_fields` argument was removed from protobuf 5.x | ||||||
| # and replaced with `always_print_fields_with_no_presence` which very similar but has | ||||||
| # handles optional fields consistently by not affecting them. | ||||||
| # The old flag accidentally had inconsistent behavior between proto2 | ||||||
| # optional and proto3 optional fields. | ||||||
| return MessageToDict( | ||||||
| cls.pb(instance), | ||||||
| always_print_fields_with_no_presence=print_fields, | ||||||
| preserving_proto_field_name=preserving_proto_field_name, | ||||||
| use_integers_for_enums=use_integers_for_enums, | ||||||
| float_precision=float_precision, | ||||||
| ) | ||||||
| return _message_to_map(map_fn=MessageToDict, **locals()) | ||||||
|
|
||||||
| def copy_from(cls, instance, other): | ||||||
| """Equivalent for protobuf.Message.CopyFrom | ||||||
|
|
@@ -966,4 +915,48 @@ def pb(self) -> Type[message.Message]: | |||||
| return self._pb | ||||||
|
|
||||||
|
|
||||||
| def _message_to_map( | ||||||
| cls, | ||||||
| map_fn, | ||||||
| instance, | ||||||
| *, | ||||||
| including_default_value_fields=None, | ||||||
| always_print_fields_with_no_presence=None, | ||||||
| float_precision=None, | ||||||
| **kwargs, | ||||||
| ): | ||||||
| """ | ||||||
| Helper for logic for Message.to_dict and Message.to_json | ||||||
| """ | ||||||
|
|
||||||
| # The `including_default_value_fields` argument was removed from protobuf 5.x | ||||||
| # and replaced with `always_print_fields_with_no_presence` which very similar but has | ||||||
| # handles optional fields consistently by not affecting them. | ||||||
| # The old flag accidentally had inconsistent behavior between proto2 | ||||||
| # optional and proto3 optional fields. | ||||||
| print_fields = cls._normalize_print_fields_without_presence( | ||||||
| always_print_fields_with_no_presence, including_default_value_fields | ||||||
| ) | ||||||
| if _PROTOBUF_MAJOR_VERSION in ("3", "4"): | ||||||
| kwargs["including_default_value_fields"] = print_fields | ||||||
| else: | ||||||
| kwargs["always_print_fields_with_no_presence"] = print_fields | ||||||
|
|
||||||
| if float_precision: | ||||||
| # float_precision removed in protobuf 7 | ||||||
| if _PROTOBUF_MAJOR_VERSION in ("3", "4", "5", "6"): | ||||||
| kwargs["float_precision"] = float_precision | ||||||
| warning_msg = "`float_precision` will be removed in Protobuf 7.x." | ||||||
|
|
||||||
| else: # pragma: NO COVER | ||||||
| warning_msg = ( | ||||||
| "`float_precision` was removed in Protobuf 7.x+, and will be ignored." | ||||||
| ) | ||||||
| warnings.warn(warning_msg, DeprecationWarning, stacklevel=3) | ||||||
| # supress similar float_precision warning from protobuf library | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated spelling:
Suggested change
|
||||||
| with warnings.catch_warnings(): | ||||||
| warnings.simplefilter("ignore", category=UserWarning) | ||||||
| return map_fn(cls.pb(instance), **kwargs) | ||||||
|
|
||||||
|
|
||||||
| __all__ = ("Message",) | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -253,13 +253,36 @@ class Squid(proto.Message): | |
| assert re.search(r"massKg.*name", j) | ||
|
|
||
|
|
||
| # TODO: https://github.com/googleapis/proto-plus-python/issues/390 | ||
| def test_json_float_precision(): | ||
| if int(proto.message._PROTOBUF_MAJOR_VERSION) >= 7: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NO ACTION: I situations like this, I often choose to decorate a test with |
||
| pytest.skip("float_precision was removed in protobuf 7.x") | ||
|
|
||
| class Squid(proto.Message): | ||
| name = proto.Field(proto.STRING, number=1) | ||
| mass_kg = proto.Field(proto.FLOAT, number=2) | ||
|
|
||
| with pytest.warns(DeprecationWarning) as warnings: | ||
| s = Squid(name="Steve", mass_kg=3.141592) | ||
| j = Squid.to_json(s, float_precision=3, indent=None) | ||
|
|
||
| assert j == '{"name": "Steve", "massKg": 3.14}' | ||
| assert len(warnings) == 1 | ||
| assert "`float_precision` will be removed" in warnings[0].message.args[0] | ||
|
|
||
|
|
||
| def test_json_float_precision_7_plus(): | ||
| if int(proto.message._PROTOBUF_MAJOR_VERSION) < 7: | ||
| pytest.skip("unsupported protobuf version for test") | ||
|
|
||
| class Squid(proto.Message): | ||
| name = proto.Field(proto.STRING, number=1) | ||
| mass_kg = proto.Field(proto.FLOAT, number=2) | ||
|
|
||
| s = Squid(name="Steve", mass_kg=3.14159265) | ||
| j = Squid.to_json(s, float_precision=3, indent=None) | ||
| s = Squid(name="Steve", mass_kg=3.141592) | ||
| with pytest.warns(DeprecationWarning) as warnings: | ||
| j = Squid.to_json(s, float_precision=3, indent=None) | ||
|
|
||
| assert j == '{"name": "Steve", "massKg": 3.141592}' | ||
|
|
||
| assert j == '{"name": "Steve", "massKg": 3.14}' | ||
| assert len(warnings) == 1 | ||
| assert "`float_precision` was removed" in warnings[0].message.args[0] | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated incorrect grammar and tweaked the wording of the closing phrase.