Skip to content

WIP: use proto-plus instead of _pb #332

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.

Sign up for GitHub

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

Closed
cguardia wants to merge 1 commit into from
Closed

WIP: use proto-plus instead of _pb #332

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 8 additions & 12 deletions google/cloud/bigquery/model.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

import copy

from google.protobuf import json_format
import json
import six

import google.cloud._helpers
Expand Down Expand Up @@ -55,7 +55,7 @@ class Model(object):
def __init__(self, model_ref):
# Use _proto on read-only properties to use it's built-in type
# conversion.
self._proto = types.Model()._pb
self._proto = types.Model()

# Use _properties on read-write properties to match the REST API
# semantics. The BigQuery API makes a distinction between an unset
Expand All @@ -67,7 +67,7 @@ def __init__(self, model_ref):
model_ref = ModelReference.from_string(model_ref)

if model_ref:
self._proto.model_reference.CopyFrom(model_ref._proto)
self._proto.model_reference = model_ref._proto

@property
def reference(self):
Expand Down Expand Up @@ -305,9 +305,7 @@ def from_api_repr(cls, resource):
start_time = datetime_helpers.from_microseconds(1e3 * float(start_time))
training_run["startTime"] = datetime_helpers.to_rfc3339(start_time)

this._proto = json_format.ParseDict(
resource, types.Model()._pb, ignore_unknown_fields=True
)
this._proto = types.Model.from_json(json.dumps(resource))
Copy link
Contributor

@tswast tswast Oct 19, 2020

Choose a reason for hiding this comment

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

Can we pass resource to the Model constructor here?

googleapis/proto-plus-python#152 (comment)

Copy link
Contributor

@tswast tswast Oct 19, 2020

Choose a reason for hiding this comment

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

We'll still want googleapis/proto-plus-python#153 to support ignore_unknown_fields, though.

Copy link
Contributor Author

@cguardia cguardia Oct 19, 2020

Choose a reason for hiding this comment

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

I already had tried that during my testing. It gives a KeyError on the metaclass during __init__, right here:

https://github.com/googleapis/proto-plus-python/blob/master/proto/message.py#L425

I was using this code:

this._proto = types.Model(resource)

Copy link
Contributor

@tswast tswast Oct 21, 2020

Choose a reason for hiding this comment

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

We should be able to update this to types.Model(resource, ignore_unknown_fields=True) now.

return this

def _build_resource(self, filter_fields):
Expand All @@ -323,7 +321,7 @@ def to_api_repr(self):
Returns:
Dict[str, object]: Model reference represented as an API resource
"""
return json_format.MessageToDict(self._proto)
return json.loads(types.Model.to_json(self._proto))


class ModelReference(object):
Expand All @@ -334,7 +332,7 @@ class ModelReference(object):
"""

def __init__(self):
self._proto = types.ModelReference()._pb
self._proto = types.ModelReference()
self._properties = {}

@property
Expand Down Expand Up @@ -377,9 +375,7 @@ def from_api_repr(cls, resource):
# Keep a reference to the resource as a workaround to find unknown
# field values.
ref._properties = resource
ref._proto = json_format.ParseDict(
resource, types.ModelReference()._pb, ignore_unknown_fields=True
)
ref._proto = types.ModelReference.from_json(json.dumps(resource))

return ref

Expand Down Expand Up @@ -418,7 +414,7 @@ def to_api_repr(self):
Returns:
Dict[str, object]: Model reference represented as an API resource
"""
return json_format.MessageToDict(self._proto)
return json.loads(types.ModelReference.to_json(self._proto))

def _key(self):
"""Unique key for this model.
Expand Down
12 changes: 5 additions & 7 deletions tests/unit/model/test_model.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,21 +123,21 @@ def test_from_api_repr(target_class):
assert got.training_runs[0].training_options.initial_learn_rate == 1.0
assert (
got.training_runs[0]
.start_time.ToDatetime()
.start_time
.replace(tzinfo=google.cloud._helpers.UTC)
== creation_time
)
assert got.training_runs[1].training_options.initial_learn_rate == 0.5
assert (
got.training_runs[1]
.start_time.ToDatetime()
.start_time
.replace(tzinfo=google.cloud._helpers.UTC)
== modified_time
)
assert got.training_runs[2].training_options.initial_learn_rate == 0.25
assert (
got.training_runs[2]
.start_time.ToDatetime()
.start_time
.replace(tzinfo=google.cloud._helpers.UTC)
== expiration_time
)
Expand Down Expand Up @@ -321,7 +321,7 @@ def test_repr(target_class):


def test_to_api_repr(target_class):
from google.protobuf import json_format
import json

model = target_class("my-proj.my_dset.my_model")
resource = {
Expand Down Expand Up @@ -357,8 +357,6 @@ def test_to_api_repr(target_class):
"kmsKeyName": "projects/1/locations/us/keyRings/1/cryptoKeys/1"
},
}
model._proto = json_format.ParseDict(
resource, types.Model()._pb, ignore_unknown_fields=True
)
model._proto = types.Model.from_json(json.dumps(resource))
got = model.to_api_repr()
assert got == resource