jrmccluskey commented on code in PR #39210:
URL: https://github.com/apache/beam/pull/39210#discussion_r3544462227
##########
sdks/python/apache_beam/typehints/opcodes.py:
##########
@@ -570,18 +571,17 @@ def gen_start(state, arg):
def load_closure(state, arg):
- # The arg is no longer offset by len(covar_names) as of 3.11
+ # closure_type performs version-aware resolution of the index: as of 3.11
+ # it refers to the frame's localsplus storage (in which the cell of a
+ # captured parameter shares the parameter's slot) rather than
+ # co_cellvars + co_freevars.
Review Comment:
If you're adding a layer of indirection to the index resolution here,
there's not really a reason to explain this detail here. Better off making sure
`closure_type()` is well-documented
##########
sdks/python/apache_beam/typehints/trivial_inference_test.py:
##########
@@ -276,7 +277,11 @@ def testClosure(self):
y = 1.0
self.assertReturnType(typehints.Tuple[int, float], lambda: (x, y))
- @unittest.skip("https://github.com/apache/beam/issues/28420";)
+ @unittest.skipIf(
+ sys.version_info >= (3, 13),
+ 'MAKE_FUNCTION closure emulation is not yet supported from Python '
+ '3.13, in which closures are attached via SET_FUNCTION_ATTRIBUTE '
+ 'after function creation: https://github.com/apache/beam/issues/28420')
Review Comment:
```suggestion
'MAKE_FUNCTION closure emulation is not yet supported after Python '
'3.12. In 3.13+, closures are attached via SET_FUNCTION_ATTRIBUTE '
'after function creation: https://github.com/apache/beam/issues/28420')
```
The phrasing here is a little confusing, let's streamline a bit. Same goes
for the other skip messages.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
