Improve annotation inference to be compatible with module reloading.

PiperOrigin-RevId: 723558958
google · Feb 5, 2025 · 65c7e52 · 65c7e52
1 parent 97d6446
commit 65c7e52
Show file tree

Hide file tree

Showing 3 changed files with 100 additions and 26 deletions.
diff --git a/pyglove/core/typing/annotation_conversion.py b/pyglove/core/typing/annotation_conversion.py
@@ -195,24 +195,64 @@ def _type_id() -> str:
     return t_id
 
   def _resolve(type_id: str):
+
+    def _as_forward_ref() -> typing.ForwardRef:
+      return typing.ForwardRef(type_id, False, parent_module)  # pytype: disable=not-callable
+
     def _resolve_name(name: str, parent_obj: typing.Any):
       if name == 'None':
-        return None
+        return None, True
       if parent_obj is not None and hasattr(parent_obj, name):
-        return getattr(parent_obj, name)
+        return getattr(parent_obj, name), False
       if hasattr(builtins, name):
-        return getattr(builtins, name)
+        return getattr(builtins, name), True
       if type_id == '...':
-        return ...
-      return utils.MISSING_VALUE
-    parent_obj = parent_module
-    for name in type_id.split('.'):
-      parent_obj = _resolve_name(name, parent_obj)
-      if parent_obj == utils.MISSING_VALUE:
-        return typing.ForwardRef(     # pytype: disable=not-callable
-            type_id, False, parent_module
-        )
-    return parent_obj
+        return ..., True
+      return utils.MISSING_VALUE, False
+
+    names = type_id.split('.')
+    if len(names) == 1:
+      reference, is_builtin = _resolve_name(names[0], parent_module)
+      if is_builtin:
+        return reference
+      if not is_builtin and (
+          # When reference is not found, we should treat it as a forward
+          # reference.
+          reference == utils.MISSING_VALUE
+          # When module is being reloaded, we should treat all non-builtin
+          # references as forward references.
+          or getattr(parent_module, '__reloading__', False)
+      ):
+        return _as_forward_ref()
+      return reference
+
+    root_obj, _ = _resolve_name(names[0], parent_module)
+    # When root object is not found, we should treat it as a forward reference.
+    if root_obj == utils.MISSING_VALUE:
+      return _as_forward_ref()
+
+    parent_obj = root_obj
+    # When root object is a module, we should treat reference to its children
+    # as non-forward references.
+    if inspect.ismodule(root_obj):
+      for name in names[1:]:
+        parent_obj, _ = _resolve_name(name, parent_obj)
+        if parent_obj == utils.MISSING_VALUE:
+          raise TypeError(f'{type_id!r} does not exist.')
+      return parent_obj
+    # When root object is non-module variable of current module, and when the
+    # module is being reloaded, we should treat reference to its children as
+    # forward references.
+    elif getattr(parent_module, '__reloading__', False):
+      return _as_forward_ref()
+    # When root object is non-module variable of current module, we should treat
+    # unresolved reference to its children as forward references.
+    else:
+      for name in names[1:]:
+        parent_obj, _ = _resolve_name(name, parent_obj)
+        if parent_obj == utils.MISSING_VALUE:
+          return _as_forward_ref()
+      return parent_obj
 
   root = _maybe_union()
   if _pos() != len(s):

diff --git a/pyglove/core/typing/annotation_conversion_test.py b/pyglove/core/typing/annotation_conversion_test.py
@@ -27,7 +27,8 @@
 
 
 class Foo:
-  pass
+  class Bar:
+    pass
 
 
 _MODULE = sys.modules[__name__]
@@ -58,6 +59,18 @@ def test_basic_types(self):
         annotation_conversion.annotation_from_str('tuple[int, str]'),
         tuple[int, str]
     )
+    self.assertEqual(
+        annotation_conversion.annotation_from_str('list[Foo]', _MODULE),
+        list[Foo]
+    )
+    self.assertEqual(
+        annotation_conversion.annotation_from_str('list[Foo.Bar]', _MODULE),
+        list[Foo.Bar]
+    )
+    self.assertEqual(
+        annotation_conversion.annotation_from_str('list[Foo.Baz]', _MODULE),
+        list[typing.ForwardRef('Foo.Baz', False, _MODULE)]
+    )
 
   def test_generic_types(self):
     self.assertEqual(
@@ -139,6 +152,28 @@ def test_forward_ref(self):
         ]
     )
 
+  def test_reloading(self):
+    setattr(_MODULE, '__reloading__', True)
+    self.assertEqual(
+        annotation_conversion.annotation_from_str(
+            'typing.List[Foo]', _MODULE),
+        typing.List[
+            typing.ForwardRef(
+                'Foo', False, _MODULE
+            )
+        ]
+    )
+    self.assertEqual(
+        annotation_conversion.annotation_from_str(
+            'typing.List[Foo.Bar]', _MODULE),
+        typing.List[
+            typing.ForwardRef(
+                'Foo.Bar', False, _MODULE
+            )
+        ]
+    )
+    delattr(_MODULE, '__reloading__')
+
   def test_bad_annotation(self):
     with self.assertRaisesRegex(SyntaxError, 'Expected type identifier'):
       annotation_conversion.annotation_from_str('typing.List[]')
@@ -152,6 +187,9 @@ def test_bad_annotation(self):
     with self.assertRaisesRegex(SyntaxError, 'Expected "]"'):
       annotation_conversion.annotation_from_str('typing.Callable[[x')
 
+    with self.assertRaisesRegex(TypeError, '.* does not exist'):
+      annotation_conversion.annotation_from_str('typing.Foo', _MODULE)
+
 
 class FieldFromAnnotationTest(unittest.TestCase):
   """Tests for Field.fromAnnotation."""

diff --git a/pyglove/core/typing/class_schema.py b/pyglove/core/typing/class_schema.py
@@ -100,7 +100,6 @@ class ForwardRef(utils.Formattable):
   def __init__(self, module: types.ModuleType, qualname: str):
     self._module = module
     self._qualname = qualname
-    self._resolved_value = None
 
   @property
   def module(self) -> types.ModuleType:
@@ -129,9 +128,7 @@ def as_annotation(self) -> Union[Type[Any], str]:
   @property
   def resolved(self) -> bool:
     """Returns True if the symbol for the name is resolved.."""
-    if self._resolved_value is None:
-      self._resolved_value = self._resolve()
-    return self._resolved_value is not None
+    return self._resolve() is not None
 
   def _resolve(self) -> Optional[Any]:
     names = self._qualname.split('.')
@@ -150,14 +147,13 @@ def _resolve(self) -> Optional[Any]:
   @property
   def cls(self) -> Type[Any]:
     """Returns the resolved reference class.."""
-    if self._resolved_value is None:
-      self._resolved_value = self._resolve()
-      if self._resolved_value is None:
-        raise TypeError(
-            f'{self.qualname!r} does not exist in '
-            f'module {self.module.__name__!r}'
-        )
-    return self._resolved_value
+    reference = self._resolve()
+    if reference is None:
+      raise TypeError(
+          f'{self.qualname!r} does not exist in '
+          f'module {self.module.__name__!r}'
+      )
+    return reference
 
   def format(
       self,