Skip to content
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

Jump to bottom

gh-119180: Add VALUE_WITH_FAKE_GLOBALS format to annotationlib #124415

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
+73 −27
Open

gh-119180: Add VALUE_WITH_FAKE_GLOBALS format to annotationlib #124415

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ddacdb7
gh-119180: Add VALUE_WITH_FAKE_GLOBALS format to annotationlib
JelleZijlstra Sep 24, 2024
5bb1d1e
Merge remote-tracking branch 'upstream/main' into pep649-fakevalue
JelleZijlstra Sep 25, 2024
ab05089
Change the values
JelleZijlstra Sep 25, 2024
3ba132a
fix constevaluator
JelleZijlstra Sep 25, 2024
26f1c7c
Merge branch 'main' into pep649-fakevalue
JelleZijlstra Sep 25, 2024
2012bb4
fix tests
JelleZijlstra Sep 26, 2024
f77be37
Merge remote-tracking branch 'upstream/main' into pep649-fakevalue
JelleZijlstra Sep 26, 2024
80818c6
Merge branch 'main' into pep649-fakevalue
JelleZijlstra Sep 28, 2024
4081f8f
Add C enum for format
JelleZijlstra Sep 29, 2024
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
11 changes: 11 additions & 0 deletions Doc/library/annotationlib.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

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

VALUE_WITH_FAKE_GLOBALS is now 2. But why publish their values in the first place?

Copy link
Member Author

Choose a reason for hiding this comment

The 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
Expand Down
13 changes: 9 additions & 4 deletions Lib/annotationlib.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -495,6 +496,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")
Copy link
Contributor

Choose a reason for hiding this comment

The 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 "The ". The text should read "VALUES_WITH_FAKE_GLOBALS format is for internal use only".

Copy link
Member Author

Choose a reason for hiding this comment

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

There is no meaningful value to return for VALUE_WITH_FAKE_GLOBALS; it's only useful within an annotate function.

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:
Expand Down Expand Up @@ -528,7 +531,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 {
Expand Down Expand Up @@ -588,7 +591,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
if isinstance(obj.__ast_node__, str):
Expand Down Expand Up @@ -708,6 +711,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}")

Expand Down
36 changes: 26 additions & 10 deletions Lib/test/test_annotationlib.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

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

See related comments, I don't think the fact that STRING format is the int object 2 is or should be part of the interface. I don't want people hard-coding it.

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):
Expand Down Expand Up @@ -413,19 +416,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})
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is format hard-coded here? I'd prefer you think of it like an enum, an arbitrary value whose actual value doesn't matter. I don't think it's necessary to externally test that the format constants are specific values; they happen to be 1, 2, 3, 4, but I don't want external users to rely on that or ever hard-code them.

Copy link
Member Author

Choose a reason for hiding this comment

The 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:
Expand Down Expand Up @@ -459,6 +471,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),
Expand Down Expand Up @@ -756,6 +770,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(
Expand Down Expand Up @@ -944,7 +960,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

Expand Down
4 changes: 2 additions & 2 deletions Lib/test/test_type_annotations.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ def test_module(self):
ns = run_code("x: undefined = 1")
anno = ns["__annotate__"]
with self.assertRaises(NotImplementedError):
anno(2)
anno(3)

with self.assertRaises(NameError):
anno(1)
Expand Down Expand Up @@ -376,7 +376,7 @@ class X:
annotate(annotationlib.Format.FORWARDREF)
with self.assertRaises(NotImplementedError):
annotate(annotationlib.Format.STRING)
with self.assertRaises(NotImplementedError):
with self.assertRaises(TypeError):
annotate(None)
self.assertEqual(annotate(annotationlib.Format.VALUE), {"x": int})

Expand Down
15 changes: 10 additions & 5 deletions Lib/typing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2934,10 +2934,13 @@ def _make_eager_annotate(types):
checked_types = {key: _type_check(val, f"field {key} annotation must be a type")
for key, val in types.items()}
def annotate(format):
if format in (annotationlib.Format.VALUE, annotationlib.Format.FORWARDREF):
return checked_types
else:
return annotationlib.annotations_to_string(types)
match format:
case annotationlib.Format.VALUE | annotationlib.Format.FORWARDREF:
return checked_types
case annotationlib.Format.STRING:
return annotationlib.annotations_to_string(types)
case _:
raise NotImplementedError(format)
return annotate


Expand Down Expand Up @@ -3227,8 +3230,10 @@ def __annotate__(format):
}
elif format == annotationlib.Format.STRING:
own = annotationlib.annotations_to_string(own_annotations)
else:
elif format in (annotationlib.Format.FORWARDREF, annotationlib.Format.VALUE):
own = own_checked_annotations
else:
raise NotImplementedError(format)
annos.update(own)
return annos

Expand Down
2 changes: 1 addition & 1 deletion Objects/typevarobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ constevaluator_call(PyObject *self, PyObject *args, PyObject *kwargs)
return NULL;
}
PyObject *value = ((constevaluatorobject *)self)->value;
if (format == 3) { // STRING
if (format == 4) { // STRING
Copy link
Contributor

Choose a reason for hiding this comment

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

The formats should be defined in C too. Macros are fine. I'm expecting FORMAT_STRING here, optionally with some variant of Py_ in front. Worst case, Py_ANNOTATE_FORMAT_STRING.

PyUnicodeWriter *writer = PyUnicodeWriter_Create(5); // cannot be <5
if (writer == NULL) {
return NULL;
Expand Down
8 changes: 5 additions & 3 deletions Python/codegen.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -655,13 +655,15 @@ codegen_setup_annotations_scope(compiler *c, location loc,
codegen_enter_scope(c, name, COMPILE_SCOPE_ANNOTATIONS,
key, loc.lineno, NULL, &umd));

// if .format != 1: raise NotImplementedError
// if .format > 2: raise NotImplementedError
_Py_DECLARE_STR(format, ".format");
PyObject *two = PyLong_FromLong(2);
ADDOP_I(c, loc, LOAD_FAST, 0);
ADDOP_LOAD_CONST(c, loc, _PyLong_GetOne());
ADDOP_I(c, loc, COMPARE_OP, (Py_NE << 5) | compare_masks[Py_NE]);
ADDOP_LOAD_CONST(c, loc, two);
ADDOP_I(c, loc, COMPARE_OP, (Py_GT << 5) | compare_masks[Py_GT]);
NEW_JUMP_TARGET_LABEL(c, body);
ADDOP_JUMP(c, loc, POP_JUMP_IF_FALSE, body);

ADDOP_I(c, loc, LOAD_COMMON_CONSTANT, CONSTANT_NOTIMPLEMENTEDERROR);
ADDOP_I(c, loc, RAISE_VARARGS, 1);
USE_LABEL(c, body);
Expand Down
Loading