Re: Implementing type hints on multi-output PTransforms

2020-03-31 Thread Joshua B. Harrison
Ok - that makes sense. My specific workaround was to remove the
with_output_types for now, so advising the user on this in the error
message would be nice. I was just worried about silently passing.

As for the formalization:

   1. I am a little confused on how this is different than passing multiple
   tagged inputs to a PTransform that does a CoGroupBy*. In this case,
   with_input_types seems to expect a union of all the types for the keyed
   values. Why would the same not work for output types?
   2. What is the process for proposing a formalized solution? Should I
   start a document, or does one already exist? Or does this kind of thing get
   tracked via Jira issues?

Best,
Joshua

On Tue, Mar 31, 2020 at 11:07 AM Udi Meiri  wrote:

> Hi Joshua,
> I've been working on type hints recently.
> Your issue is similar to this:
> https://issues.apache.org/jira/browse/BEAM-8782 (exactly the same if tags
> are passed to with_outputs() in the example).
> There's also this related bug about type inference:
> https://issues.apache.org/jira/browse/BEAM-4132
>
> I agree with Luke that it would be helpful to point to a workaround in the
> error message (such as removing with_output_types).
>
> From what I remember, we'll need to formalize how multi-output type hints
> are provided to Beam.
> For example, by passing keywords to with_output_types: main=type,
> TAG=type, etc.
>
> On Tue, Mar 31, 2020 at 9:55 AM Luke Cwik  wrote:
>
>> I can see that argument but what does a user need to do in this case if
>> we raise NotImplementedError? Would the need to disable type checking
>> everywhere?
>>
>> Over the long term users will need to deal with improvements to type
>> checking and will need to fix typing errors when they change Apache Beam
>> versions.
>>
>>
>> On Tue, Mar 31, 2020 at 9:34 AM Joshua B. Harrison <
>> josh.harri...@gmail.com> wrote:
>>
>>> 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  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
>>>>>  |  404-433-0242 <(404)%20433-0242>
>>>>>
>>>>
>>>
>>> --
>>> Joshua Harrison |  Software Engineer |  joshharri...@gmail.com
>>>  |  404-433-0242 <(404)%20433-0242>
>>>
>>

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


Re: Implementing type hints on multi-output PTransforms

2020-03-31 Thread Joshua B. Harrison
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  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
>>  |  404-433-0242 <(404)%20433-0242>
>>
>

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


Re: [VOTE + INPUT] Beam Mascot Designs, 3rd iteration - Deadline Wednesday, April 1

2020-03-30 Thread Joshua B. Harrison
On Mon, Mar 30, 2020 at 8:09 PM Julian Bruno  wrote:

> Hello Apache Beam Community,
>
> We need a third input from the community to finish the design. Please
> share your input no later than Wednesday, April 1st, at noon Pacific Time.
> Below you will find a link to the presentation of the work process and we
> are eager to know what you think of the current design [1].
>
> Our question to you:
>
> 1. Do you prefer stripes or no stripes?
>
I prefer the logo with stripes.

>
> Please reply inline, so it is clear what exactly you are referring to. The
> vote and input phase will be open until Wednesday, April 1st, at 12 pm
> Pacific Time. We will incorporate the feedback into the final design
> iteration of the mascot.
>
>
> Please find below the attached source file (.SVG) and a High-Quality Image
> (.PNG).
>
> Thank you,
>
> --
> Julian Bruno // Visual Artist & Graphic Designer
>  (510) 367-0551 / SF Bay Area, CA
> www.instagram.com/julbro.art
>
> [1]
>
>  3/30 - Mascot Weekly Update
> 
>
>
>
> ᐧ
>


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


Implementing type hints on multi-output PTransforms

2020-03-30 Thread Joshua B. Harrison
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
 |  404-433-0242