markjm opened a new pull request, #3321:
URL: https://github.com/apache/thrift/pull/3321
## Description
This change extends the type-hinted codegen to work more nicely with frozen
structs. We directly use the same logic in `member_hint` which adds type
annotations to the `__init__` to provide class-level annotations.
Using the tutorial structs:
### Before
```python
class InvalidOperation(TFrozenExceptionBase):
"""
Structs can also be exceptions, if they are nasty.
Attributes:
- whatOp
- why
"""
thrift_spec: typing.Any = None
def __init__(self, whatOp: typing.Optional[int] = None, why:
typing.Optional[str] = None,):
super(InvalidOperation, self).__setattr__('whatOp', whatOp)
super(InvalidOperation, self).__setattr__('why', why)
```
### After
```python
class InvalidOperation(TFrozenExceptionBase):
"""
Structs can also be exceptions, if they are nasty.
Attributes:
- whatOp
- why
"""
thrift_spec: typing.Any = None
whatOp: typing.Optional[int]
why: typing.Optional[str]
def __init__(self, whatOp: typing.Optional[int] = None, why:
typing.Optional[str] = None,):
super(InvalidOperation, self).__setattr__('whatOp', whatOp)
super(InvalidOperation, self).__setattr__('why', why)
```
### Diff
```diff
+ whatOp: typing.Optional[int]
+ why: typing.Optional[str]
```
## Why
Because we use `super(...).__setattr__` in frozen structs to initially set
the relevant attributes, typecheckers can't see or understand what attributes
exist on the struct when typechecking.
We the below examples:
[`ty` playground](https://play.ty.dev/677b70a7-798d-40c7-a24c-15d8821a7a8d)
```
Invalid override of method `__hash__`: Definition is incompatible with
`TFrozenBase.__hash__` (invalid-method-override) [Ln 119, Col 9]
Object of type `Self@__hash__` has no attribute `httpStatus`
(unresolved-attribute) [Ln 122, Col 17]
Object of type `Self@__hash__` has no attribute `message`
(unresolved-attribute) [Ln 123, Col 17]
Object of type `Self@__hash__` has no attribute `errorCode`
(unresolved-attribute) [Ln 124, Col 17]
Object of type `Self@__hash__` has no attribute `method`
(unresolved-attribute) [Ln 125, Col 17]
Object of type `Self@__hash__` has no attribute `info`
(unresolved-attribute) [Ln 126, Col 17]
Object of type `FrozenException` has no attribute `errorCode`
(unresolved-attribute) [Ln 138, Col 15]
```
*Note*: `ty` currently has the worst behaviour because it does not
understand attributes in `__slots__` yet. I suspect behaviour would be in line
after `astral-sh/ruff#20996`
[`pyright`
playground](https://pyright-play.net/?code=FAMwTg9gtgBALgTwA4EsB2BzGKpImOGAQTQQBoYB5JOFCNAQwBthWBjJhgZy5gBUAQtwCmACggAjAFbC2cAJQAuYDFUwA%2Buq5MIcLppgBeGKPkq1cABZgUIOFqSzFVGnUZMA2iQQBdIzAA5emFWNRgAE2EQDXUwYSQwTVEuYSYQJXMw1QAZfw8AcgBSLkNCsHyYQpMAa2FyGAxhOAY4ODBk1JAKWoR5eRgQfBge7DQYFLSAOk1tXX11H0ysuLgAVzAxoq5RYvkKqo6pzQ5ueen1RihhTQp828mpCHRRbL7QtUjozWEARySJrowXSWYRgDJZNS2GBoXTYLjoLjNNBsMTA0EUAHnE48TTgiHLJrrMYAMWYKSWYUGYBgLTao3GnXOsz0mmU%2BKyUAQ6gAbsx-I1mq12gCKLSwRT8WjErymPymmLxFZ0TShWZ2WEoZyeXyAITGKXaphs9UQlZEmCkpjk9lmjb8MCrEJLT4xNDXdSHQFSvGmwl2mGET1GfVK8XOqIwOIMcKeigoBK6H1ZKGieOQODTEDceyRNgQSJw6GwoJumlocISk2Yqw2OwOWSFgOBYJJ9lp3SZ7PqXP5sQi7AJuAUDyY46cHHqDGMmu2excRxsHxq-GpFLGk3tjNR8IAZTaqzksYZUxndfnsjM4eiAHcbHA%2B50KBBB63IdFxIPO4j1MJkb3G8WwRluEx4gJMp5zguAGECWwivhCz7puBYAMGgXCTLeKD3qIlbqohHbqFm36-nmkRHiOjJjqcNygeB1izvWi5vOyq7COueGfph957g6h79tW9Fngul7ANivB8AAogAHiirj0EIKSiIIIgUNJsm0PQSZIKcrCieO4nEpAABev4KWIykpEmLozE0WHCFA-yPjAABUDBgBgXDwZGDAoCk-DIMIElgJA7QAEQAMKofkhBQPmtgIAMx
m-uM%2B5yKFl5hNZ3apHZDkev2rnuZ57FhChvnCP5jhBSFogRVFMVxSACXgBAJljIivFwOl7yqFlljcJYjlpBQhUeV5tpjP1XCWJ6WL6biMAAHowFNM2jloOgsuoGVqAAAmJVxWPmV6RsI0aiBwXBxi%2BJVviYm5fjmsj-r5RYwUBqEViaEKXXRtaQQ2r1NrBXlhAC-iXaYuGlX6YwPYRXY9mRsHXemZDQ99mMQhRRzqGJNECf9jFLrhrG3b6ax2g9248QecAXVaFC-RBjEUHwDpwbpYn8IZrW-mp8QaWgZlKbzbVmWzAtycLIhaTpXP6RaSVoFLQui8rqtuGZSahbrSxEEKKASKs95cCVAC0K2tEge4tKsXAUpbVw8AwjSOzAoIheFvbu4dljHVklvoIMPUwLroWhwAxFbcA24K9vONQQvMB46BwIsYTR87XCu2xKVgEs0ee-g3uRIn0sp2nGdqFnTT%2B%2BEzgdYXoyDOXyeeN4ixLDMm3zP4OHsvkljW7baxcHcuH5NnucT4PxdgKXwiz-iU91-my8QvkwcQBvMA7b1EaaOgWFJLhIq4cPsej-bhgo7h0%2BNI3dLGLr6MscFJe9m3biV2g6f%2BHfdkft8xP2pC-UKb98Tb2-vQFOncAHBEgaoLyXBViOHaGLfmMlBZuCnGkeQTJ5RCiSEPEe8dx4UEvnHO2nkz5oNBKITBKtsHSzwekQhgo2gkIfkvCgPDlwQlQegxhGsWFCzYQQmynDEgenyPPRedwPYfwXr2ARWQhEMKYZregEiOFim4WvcIijgHhDUWDehGDRHqVwaBSRWgiFcNkdvRR2994REPvY6RQ1ASjWKrhKEbkPIeAAAx%2BHQCYfImg2gMBRBIGJ1RNCKMiXjBg9t3RJOOPQe8Ul7DqAyVoNBCRhA4jzH-YQOTEm3E0AGYpiTQZqA0e0OxKQvEel8WY-EE1cJlT8nwAK1V8C1TYPVGAsVwjxWwFAKAJs
GASCYBVBESIUTdROpoSInBiH5Scu08m3lyqVUCsooZIyxkTJwNM5ocyFloSWcIFZmUPGrW8eNWGK0BqzSohOfoy1VoD2%2Bn8rGtEqHXyuhjQRjIeFIKxpieRvYoWY0xCY%2BF31MTb2RRCDpyDQ5ZQ6s83ZE1ToJE9CJKE1SGBXAMIYF%2BmgoA%2BTQJoUK7E2gIHxT5PyWixFuFEMC8hhgAAswTgl8OKTnRohhQpCBAgAJV%2BI6REEClFe17IYAAjIKoVozDHioAOIST4Aq7ehgADeoUeiMrDjKR0oUAC%2BajynqSVnzZh1j6A0l4GxXCCQ06iGEJMWFkQzBAA)
*Note*: here, pyright doesn't throw an error, but if you hove over
`e.errorCode` you will see that the type is `Unbound` instead of the expected
`Optional[int]`
[`mypy`
playground](https://mypy-play.net/?gist=ae525a9b3c93f621fcee581f4d0c0f7d)
```
main.py:138: error: "FrozenException" has no attribute "errorCode"
[attr-defined]
Found 1 error in 1 file (checked 1 source file)
```
In each of those examples, un-comment out the type annotations and see the
errors go away (and/or `errorCode` be properly typed as `int | None`
```
httpStatus: Optional[int]
message: str
errorCode: Optional[int]
method: str
info: Optional[Any]
```
## Slots
I believe we use `__slots__` here for the memory saving of not carrying a
`__dict__` around on each object. The below script demonstrates that these
annotations **do not** increase the size of each created thrift object.
**Forward Warning**: the PR and description were not generated using AI tools,
but this validation script was:
```python
"""Prove that adding class-level type annotations to a __slots__ class
does not increase instance memory usage."""
import sys
from typing import Any, Optional
class WithoutAnnotations:
__slots__ = ('httpStatus', 'message', 'errorCode', 'method', 'info')
def __init__(
self,
httpStatus=None,
message="",
errorCode=None,
method="",
info=None,
):
object.__setattr__(self, 'httpStatus', httpStatus)
object.__setattr__(self, 'message', message)
object.__setattr__(self, 'errorCode', errorCode)
object.__setattr__(self, 'method', method)
object.__setattr__(self, 'info', info)
class WithAnnotations:
httpStatus: Optional[int]
message: str
errorCode: Optional[int]
method: str
info: Optional[Any]
__slots__ = ('httpStatus', 'message', 'errorCode', 'method', 'info')
def __init__(
self,
httpStatus: Optional[int] = None,
message: str = "",
errorCode: Optional[int] = None,
method: str = "",
info: Optional[Any] = None,
):
object.__setattr__(self, 'httpStatus', httpStatus)
object.__setattr__(self, 'message', message)
object.__setattr__(self, 'errorCode', errorCode)
object.__setattr__(self, 'method', method)
object.__setattr__(self, 'info', info)
def main():
a = WithoutAnnotations(httpStatus=400, message="Bad Request",
errorCode=1000, method="GET", info={"key": "value"})
b = WithAnnotations(httpStatus=400, message="Bad Request",
errorCode=1000, method="GET", info={"key": "value"})
size_a = sys.getsizeof(a)
size_b = sys.getsizeof(b)
print(f"Instance size WITHOUT annotations: {size_a} bytes")
print(f"Instance size WITH annotations: {size_b} bytes")
print(f"Difference: {size_b - size_a} bytes")
print()
assert not hasattr(a, '__dict__'), "WithoutAnnotations should not have
__dict__"
assert not hasattr(b, '__dict__'), "WithAnnotations should not have
__dict__"
print("Neither class has __dict__ (slots still prevent it)")
assert '__annotations__' not in b.__slots__, "Annotations are not in
__slots__"
assert '__annotations__' not in a.__slots__, "Annotations are not in
__slots__"
assert '__annotations__' not in b.__class__.__dict__.get('__dict__',
{}), "No per-instance __annotations__"
assert '__annotations__' in WithAnnotations.__dict__, "Annotations live
on the class object"
assert '__annotations__' not in WithoutAnnotations.__dict__,
"Unannotated class has no __annotations__"
print("Annotations live on the class, not on instances")
print(f" WithAnnotations.__annotations__ =
{WithAnnotations.__annotations__}")
print()
N = 100_000
objs_a = [
WithoutAnnotations(httpStatus=i, message=f"msg{i}", errorCode=i,
method="GET", info=None)
for i in range(N)
]
objs_b = [
WithAnnotations(httpStatus=i, message=f"msg{i}", errorCode=i,
method="GET", info=None)
for i in range(N)
]
total_a = sum(sys.getsizeof(o) for o in objs_a)
total_b = sum(sys.getsizeof(o) for o in objs_b)
print(f"Total for {N:,} instances WITHOUT annotations: {total_a:,}
bytes")
print(f"Total for {N:,} instances WITH annotations: {total_b:,}
bytes")
print(f"Difference: {total_b -
total_a:,} bytes")
if __name__ == "__main__":
main()
```
----
## Administravia
- [ ] Did you create an [Apache
Jira](https://issues.apache.org/jira/projects/THRIFT/issues/) ticket?
([Request account here](https://selfserve.apache.org/jira-account.html), not
required for trivial changes)
>I have gone ahead and requested a JIRA account to submit this as a ticket,
but IDK if yall would consider this to be trivial or non-trivial so posting
ahead of time
- [ ] 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.
--
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]
