[GitHub] [beam] robertwb commented on a diff in pull request #27536: Upgrade Beam to Cython 3.0.0

["robertwb (via GitHub)" <gi...@apache.org>]

robertwb commented on code in PR #27536:
URL: https://github.com/apache/beam/pull/27536#discussion_r1268277298


##########
sdks/python/apache_beam/runners/worker/operations.py:
##########
@@ -877,7 +877,7 @@ def setup(self, data_sampler=None):
 
       # See fn_data in dataflow_runner.py
       fn, args, kwargs, tags_and_types, window_fn = (
-          pickler.loads(self.spec.serialized_fn))
+        pickler.loads(self.spec.serialized_fn))

Review Comment:
   Wasn't the prior indentation correct?



##########
sdks/python/apache_beam/runners/worker/operations.py:
##########
@@ -812,7 +812,7 @@ def __init__(self,
 
     # See fn_data in dataflow_runner.py
     # TODO: Store all the items from spec?
-    self.fn, _, _, _, _ = (pickler.loads(self.spec.serialized_fn))
+    self.fn, _, _, _, _ = pickler.loads(self.spec.serialized_fn)

Review Comment:
   This looks like a Cython bug.



##########
sdks/python/apache_beam/runners/worker/operations.py:
##########
@@ -1124,16 +1118,17 @@ def encode_progress(value):
           remaining = current_element_progress.fraction_remaining
         assert completed is not None
         assert remaining is not None
+        coder = coders.IterableCoder(coders.FloatCoder())

Review Comment:
   Let's give this a bit more descriptive name. 



##########
.github/workflows/build_wheels.yml:
##########
@@ -269,7 +269,7 @@ jobs:
         # TODO: https://github.com/apache/beam/issues/23048
         CIBW_SKIP: "*-musllinux_*"
         CIBW_ENVIRONMENT: "SETUPTOOLS_USE_DISTUTILS=stdlib"
-        CIBW_BEFORE_BUILD: pip install cython==0.29.36 numpy && pip install --upgrade setuptools
+        CIBW_BEFORE_BUILD: pip install cython==3.0.0 numpy && pip install --upgrade setuptools

Review Comment:
   Sounds reasonable to me. We can get the fixes in now rather than later though. 



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org