Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
gh-119180: Add VALUE_WITH_FAKE_GLOBALS format to annotationlib#124415
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.
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
ddacdb75bb1d1eab050893ba132a26f1c7c2012bb4f77be3780818c64081f8f3d382ba10df40d718205fFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -144,6 +144,17 @@ Classes | ||
| The exact values of these strings may change in future versions of Python. | ||
| .. attribute:: VALUE_WITH_FAKE_GLOBALS | ||
| :value: 4 | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A wise man wrote these values in PEP 649 (https://peps.python.org/pep-0649/#overview). They are also relevant if you write an annotate function in C. But yes, we can probably do without them in Python code. | ||
| Special value used to signal that an annotate function is being | ||
| evaluated in a special environment with fake globals. When passed this | ||
| value, annotate functions should either return the same value as for | ||
| the :attr:`Format.VALUE` format, or raise :exc:`NotImplementedError` | ||
| to signal that they do not support execution in this environment. | ||
| This format is only used internally and should not be passed to | ||
| the functions in this module. | ||
| .. versionadded:: 3.14 | ||
| .. class:: ForwardRef | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -22,8 +22,9 @@ | ||
| class Format(enum.IntEnum): | ||
| VALUE = 1 | ||
| FORWARDREF = 2 | ||
| STRING = 3 | ||
| VALUE_WITH_FAKE_GLOBALS = 2 | ||
| FORWARDREF = 3 | ||
| STRING = 4 | ||
| _Union = None | ||
| @@ -513,6 +514,8 @@ def call_annotate_function(annotate, format, *, owner=None, _is_evaluate=False): | ||
| on the generated ForwardRef objects. | ||
| """ | ||
| if format == Format.VALUE_WITH_FAKE_GLOBALS: | ||
| raise ValueError("The VALUE_WITH_FAKE_GLOBALS format is for internal use only") | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was surprised to see this exception. I haven't given the matter a lot of thought, and I don't have a strongly held opinion yet. Can you tell me why we should raise this exception here? (My knee-jerk instinct: "special cases aren't special enough to break the rules.") p.s. if we do keep these assertions, please drop MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no meaningful value to return for Concretely, raising an exception here means that if a user-provided annotate function delegates to an API in annotationlib without further thought, it will raise an exception for VALUE_WITH_FAKE_GLOBALS, which is what we'd want. | ||
| try: | ||
| return annotate(format) | ||
| except NotImplementedError: | ||
| @@ -546,7 +549,7 @@ def call_annotate_function(annotate, format, *, owner=None, _is_evaluate=False): | ||
| argdefs=annotate.__defaults__, | ||
| kwdefaults=annotate.__kwdefaults__, | ||
| ) | ||
| annos = func(Format.VALUE) | ||
| annos = func(Format.VALUE_WITH_FAKE_GLOBALS) | ||
| if _is_evaluate: | ||
| return annos if isinstance(annos, str) else repr(annos) | ||
| return{ | ||
| @@ -607,7 +610,7 @@ def call_annotate_function(annotate, format, *, owner=None, _is_evaluate=False): | ||
| argdefs=annotate.__defaults__, | ||
| kwdefaults=annotate.__kwdefaults__, | ||
| ) | ||
| result = func(Format.VALUE) | ||
| result = func(Format.VALUE_WITH_FAKE_GLOBALS) | ||
| for obj in globals.stringifiers: | ||
| obj.__class__ = ForwardRef | ||
| obj.__stringifier_dict__ = None # not needed for ForwardRef | ||
| @@ -726,6 +729,8 @@ def get_annotations( | ||
| # But if we didn't get it, we use __annotations__ instead. | ||
| ann = _get_dunder_annotations(obj) | ||
| return annotations_to_string(ann) | ||
| case Format.VALUE_WITH_FAKE_GLOBALS: | ||
| raise ValueError("The VALUE_WITH_FAKE_GLOBALS format is for internal use only") | ||
| case _: | ||
| raise ValueError(f"Unsupported format{format!r}") | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -42,11 +42,14 @@ def test_enum(self): | ||
| self.assertEqual(Format.VALUE.value, 1) | ||
| self.assertEqual(Format.VALUE, 1) | ||
| self.assertEqual(Format.FORWARDREF.value, 2) | ||
| self.assertEqual(Format.FORWARDREF, 2) | ||
| self.assertEqual(Format.VALUE_WITH_FAKE_GLOBALS.value, 2) | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See related comments, I don't think the fact that | ||
| self.assertEqual(Format.VALUE_WITH_FAKE_GLOBALS, 2) | ||
| self.assertEqual(Format.STRING.value, 3) | ||
| self.assertEqual(Format.STRING, 3) | ||
| self.assertEqual(Format.FORWARDREF.value, 3) | ||
| self.assertEqual(Format.FORWARDREF, 3) | ||
| self.assertEqual(Format.STRING.value, 4) | ||
| self.assertEqual(Format.STRING, 4) | ||
| class TestForwardRefFormat(unittest.TestCase): | ||
| @@ -459,19 +462,28 @@ def f2(a: undefined): | ||
| annotationlib.get_annotations(f2, format=Format.FORWARDREF), | ||
| {"a": fwd}, | ||
| ) | ||
| self.assertEqual(annotationlib.get_annotations(f2, format=2),{"a": fwd}) | ||
| self.assertEqual(annotationlib.get_annotations(f2, format=3),{"a": fwd}) | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The intent was to test that the function accepts both the integer value and the enum. If you think it's OK to accept only the enum, I can switch to that model. | ||
| self.assertEqual( | ||
| annotationlib.get_annotations(f1, format=Format.STRING), | ||
| {"a": "int"}, | ||
| ) | ||
| self.assertEqual(annotationlib.get_annotations(f1, format=3),{"a": "int"}) | ||
| self.assertEqual(annotationlib.get_annotations(f1, format=4),{"a": "int"}) | ||
| with self.assertRaises(ValueError): | ||
| annotationlib.get_annotations(f1, format=0) | ||
| annotationlib.get_annotations(f1, format=42) | ||
| with self.assertRaises(ValueError): | ||
| annotationlib.get_annotations(f1, format=4) | ||
| with self.assertRaisesRegex( | ||
| ValueError, | ||
| r"The VALUE_WITH_FAKE_GLOBALS format is for internal use only", | ||
| ): | ||
| annotationlib.get_annotations(f1, format=Format.VALUE_WITH_FAKE_GLOBALS) | ||
| with self.assertRaisesRegex( | ||
| ValueError, | ||
| r"The VALUE_WITH_FAKE_GLOBALS format is for internal use only", | ||
| ): | ||
| annotationlib.get_annotations(f1, format=2) | ||
| def test_custom_object_with_annotations(self): | ||
| class C: | ||
| @@ -505,6 +517,8 @@ def foo(a: int, b: str): | ||
| foo.__annotations__ ={"a": "foo", "b": "str"} | ||
| for format in Format: | ||
| if format is Format.VALUE_WITH_FAKE_GLOBALS: | ||
| continue | ||
| with self.subTest(format=format): | ||
| self.assertEqual( | ||
| annotationlib.get_annotations(foo, format=format), | ||
| @@ -802,6 +816,8 @@ def __annotations__(self): | ||
| wa = WeirdAnnotations() | ||
| for format in Format: | ||
| if format is Format.VALUE_WITH_FAKE_GLOBALS: | ||
| continue | ||
| with ( | ||
| self.subTest(format=format), | ||
| self.assertRaisesRegex( | ||
| @@ -990,7 +1006,7 @@ def test_pep_695_generics_with_future_annotations_nested_in_function(self): | ||
| class TestCallEvaluateFunction(unittest.TestCase): | ||
| def test_evaluation(self): | ||
| def evaluate(format, exc=NotImplementedError): | ||
| if format != 1: | ||
| if format > 2: | ||
| raise exc | ||
| return undefined | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is internal only, maybe it should start with an _?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not quite internal-only, because external users who write their own
__annotate__functions need to care about this value.