jrmccluskey commented on PR #40757: URL: https://github.com/apache/beam/pull/40757#issuecomment-6667429936
> The contents LGTM. @jrmccluskey what do you think about this change generally? Is relaxing our checking for a type that can't be validated the right approach or do we have other options? In general I do not want us to intentionally add unvalidated permissiveness to Python type checking. There's a history of things working despite being technically incorrect because we've either intentionally or unintentionally hand-waived away type information. Users are then broken when we update the code to enforce checks on those types even if the underlying code runs. There's the separate headache of introducing a new failure case for pipelines where we should have halted the job at graph construction because of incompatible typing. If anything, I'd rather we surface the need for protocol type hints to be runtime checkable as a guide for authors. This brings me to an important question for @raams95: is the desire to change this functionality coming from the place of a specific use-case? An issue that was encountered while authoring a pipeline? Seeing a concrete use-case where using `typing.Protocol` is the obvious Pythonic choice in your code would go a long way towards justifying making changes here. -- 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]
