The current code errors out with a cryptic message around tag types in the
multi-output. Adding a NotImplementedError was just an attempt to make the
failure reason more clear.

I would be worried about trivially passing because then the user might
think they have type checking safety when they don't, which could cause
failures at later stages and might be hard to debug. Do you agree?

Best,
Joshua

On Tue, Mar 31, 2020 at 10:16 AM Luke Cwik <lc...@google.com> wrote:

> Would the NotImplementedError cause users pipeline errors or is that a
> signal to the type checking mechanism to ignore it?
> If this would cause failures I would rather make the unsupported case
> return something that would be trivially true.
>
> On Mon, Mar 30, 2020 at 12:01 PM Joshua B. Harrison <
> josh.harri...@gmail.com> wrote:
>
>> Hey all,
>>
>> I brought up an issue recently on the user forums noting issues around
>> type hints and multi-output PTransforms:
>> https://lists.apache.org/thread.html/r94bf2e43f09a290dbe87d5a8d7eedb34ea215e0bea861521cbdb0c1c%40%3Cuser.beam.apache.org%3E
>>
>> As mentioned there, I think that a NotImplementedError should be raised
>> when attaching type hints to multi-output PTransforms while the correct
>> implementation is figured out. And that a 'correct' implementation would
>> look something like the Union typehints that are expected on multi-input
>> PTransforms.
>>
>> I am happy to help out and wanted to get the discussion started around
>> what the community would like to see here. Thank you all for a great
>> product.
>>
>> Best,
>> Joshua
>>
>> --
>> Joshua Harrison |  Software Engineer |  joshharri...@gmail.com
>> <joshharri...@google.com> |  404-433-0242 <(404)%20433-0242>
>>
>

-- 
Joshua Harrison |  Software Engineer |  joshharri...@gmail.com
<joshharri...@google.com> |  404-433-0242

Reply via email to