Overload fast_plain_mtext to different return types depending on split=
#1172
Conversation
bswck-hai
commented
Sep 23, 2024
•
bswck-hai
commented
- Type checked the code.
|
@mozman Please review. |
|
Thanks for your contribution, but currently I integrate only bug fixes for more information see my latest posting #1175. |
| Union, | ||
| Optional, | ||
| Callable, | ||
| NamedTuple, | ||
| Any, | ||
| overload, |
There was a problem hiding this comment.
has to be imported from typing_extensions , not available in Python 3.9 (requires 3.11)
There was a problem hiding this comment.
typing.overload is part of PEP 484 and has always been in the typing module.
| @@ -9,11 +9,13 @@ | |||
| Iterable, | |||
| Iterator, | |||
| TYPE_CHECKING, | |||
| Literal, | |||
There was a problem hiding this comment.
Why? Literal has been there since Python 3.8, ezdxf supports 3.9+.
| # pylint: disable-next=too-many-branches | ||
| def fast_plain_mtext(text: str, split=False) -> Union[list[str], str]: | ||
| def fast_plain_mtext(text: str, split: bool = False) -> Union[list[str], str]: |
There was a problem hiding this comment.
I don't add bool for such cases, type inference by False and True, shorter signatures are better, therefore return list[str]|str .
There was a problem hiding this comment.
I'd argue that shorter signatures aren't better when compared against actually accurate signatures.
Type inference from False is Any in mypy (not mentioning it erroring in strict mode).
There was a problem hiding this comment.
therefore return
list[str]|str
Migrating to PEP 604 is a topic for a different PR.
|
Thank you, you are right on all points, but I'm afraid I don't want to deal with this or any future contributions from you. |
|
I'm very sorry to have caused you feel this way.
I agree that my replies weren't necessarily kind. Please forgive the tone I am sometimes having, I didn't intend this personally. Are we good now?
|
|
@mozman could you please reopen this? |
