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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to