Skip to content

Commit 1c68e78

Browse files
daiyippyglove authors
authored andcommitted
Decouple JSON conversion and type conversion.
- Removes the concept of json value converters, as it is no longer needed because the introduction of opaque object serialization. - Accepts tuple as the value for the `dest` argument for `pg.typing.get_converter`, and remove `pg.typing.get_first_applicable_converter`. PiperOrigin-RevId: 560116998
1 parent 4911073 commit 1c68e78

File tree

8 files changed

+17
-143
lines changed

8 files changed

+17
-143
lines changed

pyglove/core/object_utils/json_conversion.py

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
import pickle
2323
import types
2424
import typing
25-
from typing import Any, Callable, Dict, Iterable, List, Optional, Tuple, Type, TypeVar, Union
25+
from typing import Any, Dict, Iterable, List, Optional, Tuple, Type, TypeVar, Union
2626

2727
# Nestable[T] is a (maybe) nested structure of T, which could be T, a Dict
2828
# a List or a Tuple of Nestable[T]. We use a Union to fool PyType checker to
@@ -139,12 +139,6 @@ def from_json(cls, json_value, **kwargs):
139139
# Marker (as the first element of a list) for serializing tuples.
140140
TUPLE_MARKER = '__tuple__'
141141

142-
# Type converter that converts a complex type to basic JSON value type.
143-
# When this field is set by users, the converter will be invoked when a
144-
# complex value cannot be serialized by existing methods.
145-
TYPE_CONVERTER: Optional[
146-
Callable[[Type[Any]], Callable[[Any], JSONValueType]]] = None
147-
148142
# Class property that indicates whether to automatically register class
149143
# for deserialization. Subclass can override.
150144
auto_register = True
@@ -340,10 +334,6 @@ def to_json(value: Any, **kwargs) -> Any:
340334
elif value is ...:
341335
return {JSONConvertible.TYPE_NAME_KEY: 'type', 'name': 'builtins.Ellipsis'}
342336
else:
343-
if JSONConvertible.TYPE_CONVERTER is not None:
344-
converter = JSONConvertible.TYPE_CONVERTER(type(value)) # pylint: disable=not-callable
345-
if converter:
346-
return to_json(converter(value))
347337
return _OpaqueObject(value).to_json(**kwargs)
348338

349339

pyglove/core/symbolic/dict_test.py

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1838,34 +1838,6 @@ def test_schematized(self):
18381838
]))
18391839
self.assertEqual(sd.to_json_str(), '{"x": 1, "y": null}')
18401840

1841-
def test_serialization_with_converter(self):
1842-
1843-
class A:
1844-
1845-
def __init__(self, value: float):
1846-
self.value = value
1847-
1848-
def __eq__(self, other):
1849-
return isinstance(other, A) and other.value == self.value
1850-
1851-
spec = pg_typing.Dict([
1852-
('x', pg_typing.Int()),
1853-
('y', pg_typing.Object(A))
1854-
])
1855-
sd = Dict(x=1, y=A(2.0), value_spec=spec)
1856-
with self.assertRaisesRegex(
1857-
ValueError, 'Cannot encode opaque object .* with pickle'):
1858-
sd.to_json_str()
1859-
1860-
pg_typing.register_converter(A, float, convert_fn=lambda x: x.value)
1861-
pg_typing.register_converter(float, A, convert_fn=A)
1862-
1863-
self.assertEqual(sd.to_json(), {'x': 1, 'y': 2.0})
1864-
self.assertEqual(Dict.from_json(sd.to_json(), value_spec=spec), sd)
1865-
1866-
self.assertEqual(sd.to_json_str(), '{"x": 1, "y": 2.0}')
1867-
self.assertEqual(base.from_json_str(sd.to_json_str(), value_spec=spec), sd)
1868-
18691841
def test_hide_default_values(self):
18701842
sd = Dict.partial(
18711843
x=1,

pyglove/core/symbolic/list_test.py

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1577,34 +1577,6 @@ def test_schematized(self):
15771577
])))
15781578
self.assertEqual(sl.to_json_str(), '[{"x": 1, "y": null}]')
15791579

1580-
def test_serialization_with_converter(self):
1581-
1582-
class A:
1583-
1584-
def __init__(self, value: float):
1585-
self.value = value
1586-
1587-
def __eq__(self, other):
1588-
return isinstance(other, A) and other.value == self.value
1589-
1590-
spec = pg_typing.List(pg_typing.Dict([
1591-
('x', pg_typing.Int()),
1592-
('y', pg_typing.Object(A))
1593-
]))
1594-
sl = List([dict(x=1, y=A(2.0))], value_spec=spec)
1595-
with self.assertRaisesRegex(
1596-
ValueError, 'Cannot encode opaque object .* with pickle.'):
1597-
sl.to_json_str()
1598-
1599-
pg_typing.register_converter(A, float, convert_fn=lambda x: x.value)
1600-
pg_typing.register_converter(float, A, convert_fn=A)
1601-
1602-
self.assertEqual(sl.to_json(), [{'x': 1, 'y': 2.0}])
1603-
self.assertEqual(List.from_json(sl.to_json(), value_spec=spec), sl)
1604-
1605-
self.assertEqual(sl.to_json_str(), '[{"x": 1, "y": 2.0}]')
1606-
self.assertEqual(base.from_json_str(sl.to_json_str(), value_spec=spec), sl)
1607-
16081580
def test_serialization_with_tuple(self):
16091581
self.assertEqual(base.to_json_str((1, 2)), '["__tuple__", 1, 2]')
16101582
self.assertEqual(base.from_json_str('["__tuple__", 1]'), (1,))

pyglove/core/symbolic/object_test.py

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2870,21 +2870,6 @@ def from_json(cls, json_dict, *args, **kwargs):
28702870
a = self._A(Y(1), y=True)
28712871
self.assertEqual(base.from_json_str(a.to_json_str()), a)
28722872

2873-
def test_serialization_with_converter(self):
2874-
2875-
c = self._C(self._X(1), y=True)
2876-
with self.assertRaisesRegex(
2877-
ValueError, 'Cannot encode opaque object .* with pickle'):
2878-
c.to_json_str()
2879-
2880-
pg_typing.register_converter(self._X, int, convert_fn=lambda x: x.value)
2881-
pg_typing.register_converter(int, self._X, convert_fn=self._X)
2882-
2883-
self.assertEqual(
2884-
c.to_json_str(),
2885-
'{"_type": "%s", "w": 1, "x": 1, "y": true}' % self._C.type_name)
2886-
self.assertEqual(base.from_json_str(c.to_json_str()), c)
2887-
28882873
def test_hide_default_values(self):
28892874
b = self._B('foo', 1)
28902875
self.assertEqual(

pyglove/core/typing/__init__.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -356,8 +356,6 @@ class Foo(pg.Object):
356356
# Type conversion.
357357
from pyglove.core.typing.type_conversion import register_converter
358358
from pyglove.core.typing.type_conversion import get_converter
359-
from pyglove.core.typing.type_conversion import get_first_applicable_converter
360-
from pyglove.core.typing.type_conversion import get_json_value_converter
361359

362360
# Inspect helpers.
363361
from pyglove.core.typing.inspect import is_subclass

pyglove/core/typing/type_conversion.py

Lines changed: 6 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,6 @@ class _TypeConverterRegistry:
2727
def __init__(self):
2828
"""Constructor."""
2929
self._converter_list = []
30-
self._json_value_types = set(
31-
[int, float, bool, type(None), list, tuple, dict, str])
3230

3331
def register(
3432
self,
@@ -44,11 +42,7 @@ def register(
4442
):
4543
raise TypeError('Argument \'src\' and \'dest\' must be a type or '
4644
'tuple of types.')
47-
if isinstance(dest, tuple):
48-
json_value_convertible = any(d in self._json_value_types for d in dest)
49-
else:
50-
json_value_convertible = dest in self._json_value_types
51-
self._converter_list.append((src, dest, convert_fn, json_value_convertible))
45+
self._converter_list.append((src, dest, convert_fn))
5246

5347
def get_converter(
5448
self, src: Type[Any], dest: Type[Any]) -> Optional[Callable[[Any], Any]]:
@@ -58,51 +52,30 @@ def get_converter(
5852
# We may consider more efficient way to do lookup in future.
5953
# NOTE(daiyip): We do reverse lookup since usually subclass converter
6054
# is register after base class.
61-
for src_type, dest_type, converter, _ in reversed(self._converter_list):
55+
for src_type, dest_type, converter in reversed(self._converter_list):
6256
if pg_inspect.is_subclass(src, src_type):
6357
dest_types = dest_type if isinstance(dest_type, tuple) else (dest_type,)
6458
for dest_type in dest_types:
6559
if pg_inspect.is_subclass(dest_type, dest):
6660
return converter
6761
return None
6862

69-
def get_json_value_converter(
70-
self, src: Type[Any]) -> Optional[Callable[[Any], Any]]:
71-
"""Get converter from source type to a JSON simple type."""
72-
for src_type, _, converter, json_value_convertible in reversed(
73-
self._converter_list):
74-
if pg_inspect.is_subclass(src, src_type) and json_value_convertible:
75-
return converter
76-
return None
77-
7863

7964
_TYPE_CONVERTER_REGISTRY = _TypeConverterRegistry()
8065

8166

8267
def get_converter(
83-
src: Type[Any], dest: Type[Any]
68+
src: Type[Any], dest: Union[Type[Any], Tuple[Type[Any], ...]]
8469
) -> Optional[Callable[[Any], Any]]:
8570
"""Get converter from source type to destination type."""
86-
return _TYPE_CONVERTER_REGISTRY.get_converter(src, dest)
87-
88-
89-
def get_first_applicable_converter(
90-
src_type: Type[Any],
91-
dest_type: Union[Type[Any], Tuple[Type[Any], ...]]):
92-
"""Get first applicable converter."""
93-
dest_types = dest_type if isinstance(dest_type, tuple) else (dest_type,)
94-
for dest_type in dest_types:
95-
converter = get_converter(src_type, dest_type)
71+
dest_types = dest if isinstance(dest, tuple) else (dest,)
72+
for dest in dest_types:
73+
converter = _TYPE_CONVERTER_REGISTRY.get_converter(src, dest)
9674
if converter is not None:
9775
return converter
9876
return None
9977

10078

101-
def get_json_value_converter(src: Type[Any]) -> Optional[Callable[[Any], Any]]:
102-
"""Get converter from source type to a JSON simple type."""
103-
return _TYPE_CONVERTER_REGISTRY.get_json_value_converter(src)
104-
105-
10679
def register_converter(
10780
src_type: Union[Type[Any], Tuple[Type[Any], ...]],
10881
dest_type: Union[Type[Any], Tuple[Type[Any], ...]],
@@ -148,4 +121,3 @@ def _register_builtin_converters():
148121

149122

150123
_register_builtin_converters()
151-
object_utils.JSONConvertible.TYPE_CONVERTER = get_json_value_converter

pyglove/core/typing/type_conversion_test.py

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -45,18 +45,13 @@ def __init__(self, x, y):
4545

4646
self.assertIs(type_conversion.get_converter(A, str), a_converter)
4747
self.assertIs(type_conversion.get_converter(A, int), a_converter)
48-
self.assertIs(type_conversion.get_json_value_converter(A), a_converter)
4948

50-
self.assertIsNone(
51-
type_conversion.get_first_applicable_converter(A, (float, bool)))
52-
self.assertIs(
53-
type_conversion.get_first_applicable_converter(A, (float, int)),
54-
a_converter)
49+
self.assertIsNone(type_conversion.get_converter(A, (float, bool)))
50+
self.assertIs(type_conversion.get_converter(A, (float, int)), a_converter)
5551

5652
# B is a subclass of A, so A's converter applies.
5753
self.assertIs(type_conversion.get_converter(B, str), a_converter)
5854
self.assertIs(type_conversion.get_converter(B, int), a_converter)
59-
self.assertIs(type_conversion.get_json_value_converter(B), a_converter)
6055

6156
b_converter = lambda b: b.y
6257
type_conversion.register_converter(B, (str, int), b_converter)
@@ -65,13 +60,9 @@ def __init__(self, x, y):
6560
# match.
6661
self.assertIs(type_conversion.get_converter(B, str), b_converter)
6762
self.assertIs(type_conversion.get_converter(B, int), b_converter)
68-
self.assertIs(type_conversion.get_json_value_converter(B), b_converter)
6963

70-
self.assertIsNone(
71-
type_conversion.get_first_applicable_converter(B, (float, bool)))
72-
self.assertIs(
73-
type_conversion.get_first_applicable_converter(B, (float, int)),
74-
b_converter)
64+
self.assertIsNone(type_conversion.get_converter(B, (float, bool)))
65+
self.assertIs(type_conversion.get_converter(B, (float, int)), b_converter)
7566

7667
# Test generics.
7768
T = typing.TypeVar('T')
@@ -120,8 +111,6 @@ class B:
120111
vs.Union([vs.Object(B), vs.Float()]).apply(A(1)).x, 1)
121112
self.assertEqual(vs.Object(A).apply(1).x, 1)
122113
self.assertEqual(vs.Object(A).apply('foo').x, 'foo')
123-
self.assertEqual(type_conversion.get_json_value_converter(A)(A(1)), 1)
124-
self.assertIsNone(type_conversion.get_json_value_converter(B))
125114

126115

127116
class BuiltInConversionsTest(unittest.TestCase):
@@ -136,9 +125,6 @@ def test_datetime_to_int(self):
136125
now = datetime.datetime.utcfromtimestamp(timestamp)
137126
self.assertEqual(vs.Object(datetime.datetime).apply(timestamp), now)
138127
self.assertEqual(vs.Int().apply(now), timestamp)
139-
self.assertEqual(
140-
type_conversion.get_json_value_converter(datetime.datetime)(now),
141-
timestamp)
142128

143129
def test_keypath_to_str(self):
144130
"""Test built-in converter between string and KeyPath."""
@@ -151,9 +137,6 @@ def test_keypath_to_str(self):
151137
['a', 'b', 'c'])
152138
self.assertEqual(
153139
vs.Str().apply(object_utils.KeyPath.parse('a.b.c')), 'a.b.c')
154-
self.assertEqual(
155-
type_conversion.get_json_value_converter(object_utils.KeyPath)(
156-
object_utils.KeyPath.parse('a.b.c')), 'a.b.c')
157140

158141

159142
if __name__ == '__main__':

pyglove/core/typing/value_specs.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -261,8 +261,7 @@ def apply(
261261
and self.value_type is not None
262262
and not pg_inspect.is_instance(value, self.value_type)
263263
):
264-
converter = type_conversion.get_first_applicable_converter(
265-
type(value), self.value_type)
264+
converter = type_conversion.get_converter(type(value), self.value_type)
266265
if converter is None:
267266
raise TypeError(
268267
object_utils.message_on_path(
@@ -2500,8 +2499,11 @@ def _apply(self,
25002499
# tf.Tensor should be able to accept tf.Variable.
25012500
matched_candidate = None
25022501
for c in self._candidates:
2503-
if c.type_resolved and type_conversion.get_first_applicable_converter(
2504-
type(value), c.value_type) is not None:
2502+
if (
2503+
c.type_resolved
2504+
and type_conversion.get_converter(type(value), c.value_type)
2505+
is not None
2506+
):
25052507
matched_candidate = c
25062508
break
25072509

0 commit comments

Comments
 (0)