fix: remove float_precision for protobuf 7 by daniel-sanche · Pull Request #559 · googleapis/proto-plus-python · GitHub
Skip to content
This repository was archived by the owner on Feb 23, 2026. It is now read-only.

fix: remove float_precision for protobuf 7#559

Merged
daniel-sanche merged 24 commits into
mainfrom
fix_proto_7
Jan 30, 2026
Merged

fix: remove float_precision for protobuf 7#559
daniel-sanche merged 24 commits into
mainfrom
fix_proto_7

Conversation

@daniel-sanche

Copy link
Copy Markdown
Contributor

Fixes googleapis/google-cloud-python#15099

Protobuf 7 is removing the float_prevision argument. This PR ignores it when passed in 7.x+, and raises a DeprecationWarning

@daniel-sanche daniel-sanche requested a review from a team January 24, 2026 00:51
@gemini-code-assist

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request correctly addresses the removal of the float_precision argument in Protobuf 7. The approach of ignoring the argument and issuing a DeprecationWarning for newer versions is sound. The refactoring of to_json and to_dict into a shared helper function is a good improvement for code maintainability. I have one minor suggestion to simplify the logic for extracting the Protobuf major version.

Comment thread proto/message.py Outdated
Comment thread proto/message.py Outdated
@parthea

parthea commented Jan 28, 2026

Copy link
Copy Markdown
Contributor

@daniel-sanche , Please could you address the failing tests?

Comment thread proto/message.py Outdated
Comment thread proto/message.py Outdated
Comment thread tests/test_json.py Outdated

# TODO: https://github.com/googleapis/proto-plus-python/issues/390
def test_json_float_precision():
if int(proto.message._PROTOBUF_MAJOR_VERSION) >= 7:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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.mark.parametrize() and include one or more happy paths and sad paths (labeled with IDs for clarity when looking at the pytest results) rather than writing two separate tests. Avoids a lot of duplicate code. Not saying you need to do anything here. Just pointing it out as an option for future tests down the road.

@daniel-sanche daniel-sanche Jan 30, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I think they started out different, but the code is pretty similar now. I'll take another look

@chalmerlowe chalmerlowe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with suggested fixes for minor spelling and grammar errors.

Also, includes a suggestion for future work, that is not required to be incorporated here (NO ACTION).

Comment thread proto/message.py Outdated

@parthea parthea Jan 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One downside of this approach is that we will no longer see new user warnings. For example, in October 2025, https://pypi.org/project/protobuf/6.33.0/ included a new deprecation warning for the float_precision argument, within the same major version. See the release notes for https://github.com/protocolbuffers/protobuf/releases/tag/v33.0-rc1

Can we use warnings.filterwarnings to narrow down the warnings that are filtered to the one that includes the text float_precision?

warnings.filterwarnings("ignore", message=".*float_precision.*")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, fixed

Comment thread pytest.ini Outdated
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
@daniel-sanche daniel-sanche merged commit 390b9d5 into main Jan 30, 2026
34 checks passed
@daniel-sanche daniel-sanche deleted the fix_proto_7 branch January 30, 2026 19:31
daniel-sanche added a commit that referenced this pull request Feb 2, 2026
PR created by the Librarian CLI to initialize a release. Merging this PR
will auto trigger a release.

Librarian Version: v1.0.1
Language Image:
us-central1-docker.pkg.dev/cloud-sdk-librarian-prod/images-prod/python-librarian-generator@sha256:d7caef319a25d618e20ba798b103434700bfd80015f525802d87621ca2528c90
<details><summary>proto-plus: 1.27.1</summary>

##
[1.27.1](v1.27.0...v1.27.1)
(2026-01-30)

### Bug Fixes

* remove float_precision for protobuf 7 (#559)
([390b9d5](390b9d57))

</details>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deprecate float_precision argument

3 participants