-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Use correct default type for str #8623
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
2622f40 to
ad1b601
Compare
|
I'm not quite sure this change is correct, since there's a distinct difference between the field being unset and being explicitly set to an empty string. For example, when reading from C++ a string that was set to Perhaps instead of changing the default value, the type should be changed to |
BUT im not 100% sure, it did not have a negative impact on my code base. |
ad1b601 to
c6101da
Compare
At least for the flatbuffer schema, strings are optional unless explicitly marked as FWIW I expect that arrays (represented in Python as lists) would run into the same issues. For example, with monster_test_generated.py I expect |
Otherwise mypy will correctly flag code like this def __init__(self): self.fooBar = None # type: Optional[str] error: Incompatible types in assignment (expression has type "None", variable has type "str")
c6101da to
ed4b4dc
Compare
|
I updated the MR and made Lists also optional, anything else needed here before it can be merged? @akb825 @dbaileychess |
|
Looks good on my end. For an official approval, you may have more luck with @aardappel assuming that @dbaileychess is still on a hiatus. |
|
Yes @akb825 is correct, we should distinguish between no string and empty string. Some language APIs conflate the two for simplicity, but FlatBuffers represents them differently. |
Otherwise mypy will correctly flag code like this
error: Incompatible types in assignment (expression has type "None", variable has type "str")