fix review idea

Signed-off-by: Manjusaka <me@manjusaka.me>

Author: Zheaoli

Doc/library/annotationlib.rst

@@ -371,6 +371,7 @@ Functions
    * For :class:`functools.partial` and :class:`functools.partialmethod` objects,
      only returns annotations for parameters that have not been bound by the
      partial application, along with the return annotation if present.
+     See :ref:`below <functools-objects-annotations>` for details.
 
    *eval_str* controls whether or not values of type :class:`!str` are
    replaced with the result of calling :func:`eval` on those values:
@@ -409,20 +410,7 @@ Functions
       >>> get_annotations(f)
       {'a': <class 'int'>, 'b': <class 'str'>, 'return': <class 'float'>}
 
-   :func:`!get_annotations` also works with :class:`functools.partial` and
-   :class:`functools.partialmethod` objects, returning only the annotations
-   for parameters that have not been bound:
-
-   .. doctest::
-
-      >>> from functools import partial
-      >>> def add(a: int, b: int, c: int) -> int:
-      ...     return a + b + c
-      >>> add_10 = partial(add, 10)
-      >>> get_annotations(add_10)
-      {'b': <class 'int'>, 'c': <class 'int'>, 'return': <class 'int'>}
-
-   .. versionadded:: 3.15
+   .. versionadded:: next
 
 .. function:: type_repr(value)
 
@@ -438,6 +426,7 @@ Functions
 
    .. versionadded:: 3.14
 
+.. _functools-objects-annotations:
 
 Using :func:`!get_annotations` with :mod:`functools` objects
 --------------------------------------------------------------
@@ -495,6 +484,36 @@ This behavior ensures that :func:`get_annotations` returns annotations that
 accurately reflect the signature of the partial or partialmethod object, as
 determined by :func:`inspect.signature`.
 
+If :func:`!get_annotations` cannot reliably determine which parameters are bound
+(for example, if :func:`inspect.signature` raises an error), it will raise a
+:exc:`TypeError` rather than returning incorrect annotations. This ensures that
+you either get correct annotations or a clear error, never incorrect annotations:
+
+.. doctest::
+
+   >>> from functools import partial
+   >>> import inspect
+   >>> def func(a: int, b: str) -> bool:
+   ...     return True
+   >>> partial_func = partial(func, 1)
+   >>> # Simulate a case where signature inspection fails
+   >>> original_sig = inspect.signature
+   >>> def broken_signature(obj):
+   ...     if isinstance(obj, partial):
+   ...         raise ValueError("Cannot inspect signature")
+   ...     return original_sig(obj)
+   >>> inspect.signature = broken_signature
+   >>> try:
+   ...     get_annotations(partial_func)
+   ... except TypeError as e:
+   ...     print(f"Got expected error: {e}")
+   ... finally:
+   ...     inspect.signature = original_sig
+   Got expected error: Cannot compute annotations for ...: unable to determine signature
+
+This design prevents the common error of returning annotations that include
+parameters which have already been bound by the partial application.
+
 
 Recipes
 -------

Doc/library/functools.rst

@@ -825,3 +825,16 @@ have three read-only attributes:
 callable, weak referenceable, and can have attributes.  There are some important
 differences.  For instance, the :attr:`~definition.__name__` and :attr:`~definition.__doc__` attributes
 are not created automatically.
+
+However, :class:`partial` objects do support the :attr:`~object.__annotate__` protocol for
+annotation introspection. When accessed, :attr:`!__annotate__` returns only the annotations
+for parameters that have not been bound by the partial application, along with the return
+annotation. This behavior is consistent with :func:`inspect.signature` and allows tools like
+:func:`annotationlib.get_annotations` to work correctly with partial objects.  See the
+:mod:`annotationlib` module documentation for more information on working with annotations
+on partial objects.
+
+:class:`partialmethod` objects similarly support :attr:`~object.__annotate__` for unbound methods.
+
+.. versionadded:: next
+   Added :attr:`~object.__annotate__` support to :class:`partial` and :class:`partialmethod`.

Lib/functools.py

@@ -541,9 +541,14 @@ def _partial_annotate(partial_obj, format):
     # Get the signature to determine which parameters are bound
     try:
         sig = inspect.signature(partial_obj)
-    except (ValueError, TypeError):
-        # If we can't get signature, return empty dict
-        return {}
+    except (ValueError, TypeError) as e:
+        # If we can't get signature, we can't reliably determine which
+        # parameters are bound. Raise an error rather than returning
+        # incorrect annotations.
+        raise TypeError(
+            f"Cannot compute annotations for {partial_obj!r}: "
+            f"unable to determine signature"
+        ) from e
 
     # Build new annotations dict with only unbound parameters
     # (parameters first, then return)
@@ -608,33 +613,52 @@ def _partialmethod_annotate(partialmethod_obj, format):
         # So if func is (self, a, b, c) and partialmethod.args=(1,)
         # Then 'self' stays, 'a' is bound, 'b' and 'c' remain
 
+        # We need to account for Placeholders which create "holes"
+        # For example: partialmethod(func, 1, Placeholder, 3) binds 'a' and 'c' but not 'b'
+
         remaining_params = func_params[1:]
-        num_positional_bound = len(partial_args)
+
+        # Track which positions are filled by Placeholder
+        placeholder_positions = set()
+        for i, arg in enumerate(partial_args):
+            if arg is Placeholder:
+                placeholder_positions.add(i)
+
+        # Number of non-Placeholder positional args
+        # This doesn't directly tell us which params are bound due to Placeholders
 
         for i, param_name in enumerate(remaining_params):
-            # Skip if this param is bound positionally
-            if i < num_positional_bound:
+            # Check if this position has a Placeholder
+            if i in placeholder_positions:
+                # This parameter is deferred by Placeholder, keep it
+                if param_name in func_annotations:
+                    new_annotations[param_name] = func_annotations[param_name]
                 continue
 
-            # For keyword binding: keep the annotation (keyword sets default, doesn't remove param)
-            if param_name in partial_keywords:
+            # Check if this position is beyond the partial_args
+            if i >= len(partial_args):
+                # This parameter is not bound at all, keep it
                 if param_name in func_annotations:
                     new_annotations[param_name] = func_annotations[param_name]
                 continue
 
-            # This parameter is not bound, keep its annotation
-            if param_name in func_annotations:
-                new_annotations[param_name] = func_annotations[param_name]
+            # Otherwise, this position is bound (not a Placeholder and within bounds)
+            # Skip it
 
         # Add return annotation at the end
         if 'return' in func_annotations:
             new_annotations['return'] = func_annotations['return']
 
         return new_annotations
 
-    except (ValueError, TypeError):
-        # If we can't process, return the original annotations
-        return func_annotations
+    except (ValueError, TypeError) as e:
+        # If we can't process the signature, we can't reliably determine
+        # which parameters are bound. Raise an error rather than returning
+        # incorrect annotations (which would include bound parameters).
+        raise TypeError(
+            f"Cannot compute annotations for {partialmethod_obj!r}: "
+            f"unable to determine which parameters are bound"
+        ) from e
 
 ################################################################################
 ### LRU Cache function decorator

Lib/test/test_annotationlib.py

@@ -1783,6 +1783,43 @@ def method(self, a, b, c):
         result = get_annotations(MyClass.partial_method)
         self.assertEqual(result, {})
 
+    def test_partialmethod_with_placeholder(self):
+        """Test partialmethod with Placeholder."""
+        class MyClass:
+            def method(self, a: int, b: str, c: float) -> bool:
+                return True
+
+            # Bind 'a', defer 'b', bind 'c'
+            partial_method = functools.partialmethod(method, 1, functools.Placeholder, 3.0)
+
+        result = get_annotations(MyClass.partial_method)
+
+        # 'self' stays, 'a' and 'c' are bound, 'b' remains
+        # For unbound partialmethod, we expect 'self' if annotated, plus remaining params
+        # Since 'self' isn't annotated, only 'b' and 'return' remain
+        self.assertIn('b', result)
+        self.assertIn('return', result)
+        self.assertNotIn('a', result)
+        self.assertNotIn('c', result)
+
+    def test_partialmethod_with_multiple_placeholders(self):
+        """Test partialmethod with multiple Placeholders."""
+        class MyClass:
+            def method(self, a: int, b: str, c: float, d: list) -> bool:
+                return True
+
+            # Bind 'a', defer 'b', defer 'c', bind 'd'
+            partial_method = functools.partialmethod(method, 1, functools.Placeholder, functools.Placeholder, [])
+
+        result = get_annotations(MyClass.partial_method)
+
+        # 'b' and 'c' remain unbound, 'a' and 'd' are bound
+        self.assertIn('b', result)
+        self.assertIn('c', result)
+        self.assertIn('return', result)
+        self.assertNotIn('a', result)
+        self.assertNotIn('d', result)
+
 
 class TestFunctoolsPartial(unittest.TestCase):
     """Tests for get_annotations() with functools.partial objects."""
@@ -1883,6 +1920,62 @@ def foo(a: int, b: str) -> bool:
         expected = {'b': str, 'return': bool}
         self.assertEqual(result, expected)
 
+    def test_partial_with_placeholder(self):
+        """Test partial with Placeholder for deferred argument."""
+        def foo(a: int, b: str, c: float) -> bool:
+            return True
+
+        # Placeholder in the middle: bind 'a', defer 'b', bind 'c'
+        partial_foo = functools.partial(foo, 1, functools.Placeholder, 3.0)
+        result = get_annotations(partial_foo)
+
+        # Only 'b' remains unbound (Placeholder defers it), 'a' and 'c' are bound
+        # So we should have 'b' and 'return'
+        expected = {'b': str, 'return': bool}
+        self.assertEqual(result, expected)
+
+    def test_partial_with_multiple_placeholders(self):
+        """Test partial with multiple Placeholders."""
+        def foo(a: int, b: str, c: float, d: list) -> bool:
+            return True
+
+        # Bind 'a', defer 'b', defer 'c', bind 'd'
+        partial_foo = functools.partial(foo, 1, functools.Placeholder, functools.Placeholder, [])
+        result = get_annotations(partial_foo)
+
+        # 'b' and 'c' remain unbound, 'a' and 'd' are bound
+        expected = {'b': str, 'c': float, 'return': bool}
+        self.assertEqual(result, expected)
+
+    def test_partial_placeholder_at_start(self):
+        """Test partial with Placeholder at the start."""
+        def foo(a: int, b: str, c: float) -> bool:
+            return True
+
+        # Defer 'a', bind 'b' and 'c'
+        partial_foo = functools.partial(foo, functools.Placeholder, "hello", 3.0)
+        result = get_annotations(partial_foo)
+
+        # Only 'a' remains unbound
+        expected = {'a': int, 'return': bool}
+        self.assertEqual(result, expected)
+
+    def test_nested_partial_with_placeholder(self):
+        """Test nested partial applications with Placeholder."""
+        def foo(a: int, b: str, c: float, d: list) -> bool:
+            return True
+
+        # First partial: bind 'a', defer 'b', bind 'c'
+        # (can't have trailing Placeholder)
+        partial1 = functools.partial(foo, 1, functools.Placeholder, 3.0)
+        # Second partial: provide 'b'
+        partial2 = functools.partial(partial1, "hello")
+        result = get_annotations(partial2)
+
+        # 'a', 'b', and 'c' are bound, only 'd' remains
+        expected = {'d': list, 'return': bool}
+        self.assertEqual(result, expected)
+
 
 class TestAnnotationLib(unittest.TestCase):
     def test__all__(self):