gh-119180: Add VALUE_WITH_FAKE_GLOBALS format to annotationlib by JelleZijlstra · Pull Request #124415 · python/cpython · GitHub

Refer to #3991.

Issue: Implement PEP 649 and PEP 749 #119180

📚 Documentation preview 📚: https://cpython-previews--124415.org.readthedocs.build/

Mostly this is about "the actual runtime values of the format constants shouldn't be defined as part of the API". I invite you to convince me otherwise.

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

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.

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.

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.

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".

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.

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.

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.

I added a C enum for these.

Since this is internal only, maybe it should start with an _?

It's not quite internal-only, because external users who write their own __annotate__ functions need to care about this value.

Suggested change

      
                PyObject *two = PyLong_FromLong(_Py_ANNOTATE_FORMAT_VALUE_WITH_FAKE_GLOBALS);
          
                PyObject *fake_globals = PyLong_FromLong(_Py_ANNOTATE_FORMAT_VALUE_WITH_FAKE_GLOBALS);

Or something, but not two.

Good point, changing to value_with_fake_globals.

…ython#124415)

pythongh-119180 : Add VALUE_WITH_FAKE_GLOBALS format to annotationlib

ddacdb7

JelleZijlstra added the skip news label Sep 24, 2024

bedevere-app Bot mentioned this pull request Sep 24, 2024

Implement PEP 649 and PEP 749 #119180

Closed

30 tasks

JelleZijlstra added 6 commits September 25, 2024 12:10

Merge remote-tracking branch 'upstream/main' into pep649-fakevalue

5bb1d1e

Change the values

ab05089

fix constevaluator

3ba132a

Merge branch 'main' into pep649-fakevalue

26f1c7c

fix tests

2012bb4

Merge remote-tracking branch 'upstream/main' into pep649-fakevalue

f77be37

JelleZijlstra marked this pull request as ready for review September 26, 2024 04:18

JelleZijlstra requested review from AlexWaygood, iritkatriel and markshannon as code owners September 26, 2024 04:18

bedevere-app Bot added the awaiting core review label Sep 26, 2024

JelleZijlstra requested a review from larryhastings September 26, 2024 04:18

Merge branch 'main' into pep649-fakevalue

80818c6

larryhastings reviewed Sep 29, 2024

View reviewed changes

Add C enum for format

4081f8f

JelleZijlstra mentioned this pull request Oct 15, 2024

Incorrect format passed to __annotate__ function #125507

Closed

Merge remote-tracking branch 'upstream/main' into pep649-fakevalue

3d382ba

iritkatriel reviewed Nov 20, 2024

View reviewed changes

larryhastings approved these changes Nov 26, 2024

View reviewed changes

bedevere-app Bot added awaiting merge and removed awaiting core review labels Nov 26, 2024

JelleZijlstra added 2 commits November 26, 2024 07:12

Merge remote-tracking branch 'upstream/main' into pep649-fakevalue

10df40d

Rename poorly named variable

718205f

JelleZijlstra enabled auto-merge (squash) November 26, 2024 15:30

JelleZijlstra merged commit dcf6292 into python:main Nov 26, 2024

bedevere-app Bot removed the awaiting merge label Nov 26, 2024

ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025

pythongh-119180 : Add VALUE_WITH_FAKE_GLOBALS format to annotationlib (p…

d24437b

…ython#124415)

Daraan mentioned this pull request Apr 1, 2025

Unit tests fails under Python 3.14 python/typing_extensions#564

Closed

                     The exact values of these strings may change in future versions of Python.
+                 .. attribute:: VALUE_WITH_FAKE_GLOBALS
+                    :value: 4

                           {"a": fwd},
                       )
-                      self.assertEqual(annotationlib.get_annotations(f2, format=2), {"a": fwd})
+                      self.assertEqual(annotationlib.get_annotations(f2, format=3), {"a": fwd})

                   """
+                  if format == Format.VALUE_WITH_FAKE_GLOBALS:
+                      raise ValueError("The VALUE_WITH_FAKE_GLOBALS format is for internal use only")

-                      self.assertEqual(Format.FORWARDREF.value, 2)
-                      self.assertEqual(Format.FORWARDREF, 2)
+                      self.assertEqual(Format.VALUE_WITH_FAKE_GLOBALS.value, 2)

                   }
                   PyObject *value = ((constevaluatorobject *)self)->value;
-                  if (format == 3) { // STRING
+                  if (format == 4) { // STRING


		The exact values of these strings may change in future versions of Python.

		.. attribute:: VALUE_WITH_FAKE_GLOBALS

                                           key, loc.lineno, NULL, &umd));
+                  // if .format > VALUE_WITH_FAKE_GLOBALS: raise NotImplementedError
+                  PyObject *two = PyLong_FromLong(_Py_ANNOTATE_FORMAT_VALUE_WITH_FAKE_GLOBALS);

	PyObject *two = PyLong_FromLong(_Py_ANNOTATE_FORMAT_VALUE_WITH_FAKE_GLOBALS);
	PyObject *fake_globals = PyLong_FromLong(_Py_ANNOTATE_FORMAT_VALUE_WITH_FAKE_GLOBALS);

gh-119180: Add VALUE_WITH_FAKE_GLOBALS format to annotationlib by JelleZijlstra · Pull Request #124415 · python/cpython · GitHub

Uh oh!

Uh oh!

gh-119180: Add VALUE_WITH_FAKE_GLOBALS format to annotationlib - #124415

gh-119180: Add VALUE_WITH_FAKE_GLOBALS format to annotationlib#124415JelleZijlstra merged 12 commits intopython:mainpython/cpython:mainfrom JelleZijlstra:pep649-fakevalueJelleZijlstra/cpython:pep649-fakevalueCopy head branch name to clipboard

Conversation

JelleZijlstra commented Sep 24, 2024 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

larryhastings left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gh-119180: Add VALUE_WITH_FAKE_GLOBALS format to annotationlib#124415
JelleZijlstra merged 12 commits into
python:mainfrom
JelleZijlstra:pep649-fakevalue

JelleZijlstra commented Sep 24, 2024 •
edited by github-actions Bot
Loading