hjtran commented on PR #35881: URL: https://github.com/apache/beam/pull/35881#issuecomment-3193875703
I feel that it'd be better for users to be explicit. Since the documentation states that partitions should be integers between 0 and max partitions, I think it's a little surprising if bools worked (even if they are technically ints, which I actually didn't know until I wrote this PR) On Sat, Aug 16, 2025, 10:13 AM liferoad ***@***.***> wrote: > ***@***.**** commented on this pull request. > ------------------------------ > > In sdks/python/apache_beam/transforms/core.py > <https://github.com/apache/beam/pull/35881#discussion_r2280446831>: > > > + if isinstance(partition, bool) or not isinstance(partition, int): > + raise ValueError( > + f"PartitionFn yielded a '{type(partition).__name__}' " > + "when it should only yields integers") > > I thought about this. I am wondering whether we could do partition = > int(partition) although partition is bool, this could make the changes > less aggressive. > > — > Reply to this email directly, view it on GitHub > <https://github.com/apache/beam/pull/35881#discussion_r2280446831>, or > unsubscribe > <https://github.com/notifications/unsubscribe-auth/ACQJSFD5MQKXE3DOJWDFBKD3N4375AVCNFSM6AAAAACD73EIWKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTCMRVHA3TQMZRGU> > . > You are receiving this because you were mentioned.Message ID: > ***@***.***> > -- 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-unsubscr...@beam.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
