THRIFT-5139/THRIFT-4181: Python3 type hints

[arkuhn]

Add type hinting (with native enums (IntEnum)) for python generated code

1. Squashed and rebased changes from stale PR
2. Removed the controversial runtime type check as it goes beyond strictly annotations
3. Added type_hints as a default option to RunClientServer.py and generate.cmake.
4. Enabled testing against thrift test suite
5. Made enum regeneration explicitly required instead of implied when this flag is provided
6. Added python tests to verify basic annotations are getting added
7. Generate a thrift_spec attribute variable declaration for all generated thrift structures and optionally type hint it if type_hints are enabled Note: Appveyor x86 / Python 3.5 build is failing since this PR generates variable type hints (x: int = 1), which are only python 3.6 and up. Do we have any patterns established for skipping tests based on platform/python version?

Example difference in generated code after running thrift --gen py:type_hints against:

struct OneOfEachZZ {
  1: required bool im_true,
  2: bool im_false,
  3: byte a_bite,
  4: i16 integer16,
  5: i32 integer32,
  6: i64 integer64,
  7: double double_precision,
  8: string some_characters,
  9: string zomg_unicode,
  10: optional bool what_who
}

service UnderscoreSrv {
  i64 some_rpc_call(1: string message, 2: OneOfEachZZ sample_struct)
}

ttypes.py

root@37314fef4537:/thrift/src/test# diff gen-py/FullCamelTest/ttypes.py gen-py-old/FullCamelTest/ttypes.py 
6c6
< #  options string: py
---
> #  options string: py:type_hints
8a9,10
> from __future__ import annotations
> import typing
12a15
> from enum import IntEnum
37,47c40,50
<     def __init__(self, im_true = None, im_false = None, a_bite = None, integer16 = None, integer32 = None, integer64 = None, double_precision = None, some_characters = None, zomg_unicode = None, what_who = None,):
<         self.im_true = im_true
<         self.im_false = im_false
<         self.a_bite = a_bite
<         self.integer16 = integer16
<         self.integer32 = integer32
<         self.integer64 = integer64
<         self.double_precision = double_precision
<         self.some_characters = some_characters
<         self.zomg_unicode = zomg_unicode
<         self.what_who = what_who
---
>     def __init__(self, im_true: bool = None, im_false: typing.Optional[bool] = None, a_bite: typing.Optional[int] = None, integer16: typing.Optional[int] = None, integer32: typing.Optional[int] = None, integer64: typing.Optional[int] = None, double_precision: typing.Optional[float] = None, some_characters: typing.Optional[str] = None, zomg_unicode: typing.Optional[str] = None, what_who: typing.Optional[bool] = None,):
>         self.im_true: bool = im_true
>         self.im_false: typing.Optional[bool] = im_false
>         self.a_bite: typing.Optional[int] = a_bite
>         self.integer16: typing.Optional[int] = integer16
>         self.integer32: typing.Optional[int] = integer32
>         self.integer64: typing.Optional[int] = integer64
>         self.double_precision: typing.Optional[float] = double_precision
>         self.some_characters: typing.Optional[str] = some_characters
>         self.zomg_unicode: typing.Optional[str] = zomg_unicode
>         self.what_who: typing.Optional[bool] = what_who

UnderscoreSrv.py

root@37314fef4537:/thrift/src/test# diff gen-py/FullCamelTest/UnderscoreSrv.py gen-py-old/FullCamelTest/UnderscoreSrv.py 
6c6
< #  options string: py
---
> #  options string: py:type_hints
8a9,10
> from __future__ import annotations
> import typing
12a15
> from enum import IntEnum
23c26
<     def some_rpc_call(self, message, sample_struct):
---
>     def some_rpc_call(self, message: str, sample_struct: OneOfEachZZ) -> int:
40c43
<     def some_rpc_call(self, message, sample_struct):
---
>     def some_rpc_call(self, message: str, sample_struct: OneOfEachZZ) -> int:
50c53
<     def send_some_rpc_call(self, message, sample_struct):
---
>     def send_some_rpc_call(self, message: str, sample_struct: OneOfEachZZ):
59c62
<     def recv_some_rpc_call(self):
---
>     def recv_some_rpc_call(self) -> int:
137,139c140,142
<     def __init__(self, message = None, sample_struct = None,):
<         self.message = message
<         self.sample_struct = sample_struct
---
>     def __init__(self, message: typing.Optional[str] = None, sample_struct: typing.Optional[OneOfEachZZ] = None,):
>         self.message: typing.Optional[str] = message
>         self.sample_struct: typing.Optional[OneOfEachZZ] = sample_struct
212,213c215,216
<     def __init__(self, success = None,):
<         self.success = success
---
>     def __init__(self, success: typing.Optional[int] = None,):
>         self.success: typing.Optional[int] = success

Unrelated to these changes, I noticed that even if a field is marked as required, declare_argument generates a default value in the function arguments of None.

Open questions: Since the original PR opened,

• [ ] Did you create an Apache Jira ticket? (Request account here, not required for trivial changes)
• [x] If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"?
• [x] Did you squash your changes to a single commit? (not required, but preferred)
• [x] Did you do your best to avoid breaking changes? If one was needed, did you label the Jira ticket with "Breaking-Change"?
• [ ] If your change does not involve any code, include [skip ci] anywhere in the commit message to free up build resources.

[salomon-smekecohen]

It would be great to have this, it would almost enable transpilation via mypyc. I believe the only missing piece would be to add to all structures:

thrift_spec: Any

[Jens-G]

@arkuhn According to ASF rules you cannot simply take other peoples code and contribute it., The original author (Konstantin Pozdniakov) can contribute himself.

[arkuhn]

@arkuhn According to ASF rules you cannot simply take other peoples code and contribute it., The original author (Konstantin Pozdniakov) can contribute himself.

I see @Jens-G. Does this mean we can not add type hints for thrift generated python libs until the original author returns? The original author hasn't commented on the original PR in 2 years.

Similarly, if he does return can he include the changes I've made (mostly testing changes, but also functional changes) in his branch or is that prohibited too?

I'm happy to accommodate whatever steps are required to get the functionality added.

[HIRANO-Satoshi]

@cspwizard Have you read this?

[cspwizard]

@cspwizard Have you read this?

Hi! Now I did. Feel free to use my code. If there is anything I can do to help - let me know.

There was an issue with my PR about duck typing. I've added some constraints to check the type of the complex property, to prevent garbage (something not defined by the contract) in the serialized message and we couldn't come to agreement on that with maintainers.

[salomon-smekecohen]

So it seems we have permission from the original contributor, as well as @arkuhn 's changes having its comments addressed.

Is all that is missing that this PR be rebased? What can I do to help get this across the line?