Improve type hints on public interface #583
Conversation
drawBot/drawBotDrawingTools.py
Outdated
| self._addInstruction("writingDirection", direction) | ||
|
|
||
| def openTypeFeatures(self, *args: bool | None, **features: bool) -> dict[str, bool]: | ||
| def openTypeFeatures( |
There was a problem hiding this comment.
I am sure we discussed this already, but why do we need args here? From the examples is not evident to me...
There was a problem hiding this comment.
Perhaps it's time to remove it. It's for bw compat of an earlier call signature. See the implementation.
There was a problem hiding this comment.
I assume you refer to openTypeFeatures(None). @typemytype what do you think?
| listOpenTypeFeatures.__doc__ = FormattedString.listOpenTypeFeatures.__doc__ | ||
|
|
||
| def fontVariations(self, *args: None, **axes: float | bool): | ||
| def fontVariations( |
There was a problem hiding this comment.
- there was a specific reason to also have a
boolon the**axes? - is
argsneeded?
There was a problem hiding this comment.
bool: I think it's for the resetVariations case. See the implementation. It's an odd call signature.
args here is also for bw compatibility. Perhaps time to remove.
Perhaps this should be rewritten like this:
def fontVariations(self, *, resetVariations: bool = False, **axes: float) -> dict[str, float]:
...I bet the current version predates the "*" feature that says "only keyword args follow". That's the main point: we want to ensure resetVariations is passed as a keyword argument.
methods affected: - `lineCap` - `underline` - `strikethrough` - `writingDirection` - `text` - `textBox` - `textBoxBaselines`
|
morning! @typemytype could you have a look at the PR? thanks : ) |
|
I think you should first make sure that the CI doesn't fail. There's currently a mypy error. |
|
Sure, I'll take care of it. I am curious to know what @typemytype thinks of the open discussions : ) |
2 participants
Hey!
I've recently noticed a couple of issues with hints:
dict[str, something]pyrightdoes infer most of them butmypyinstead defaults toAny, so better specify them)I've also left a few comments on the code on hints/validations that I'd like an input from you. I'll try to expose them as discussions here in the PR thread.
I propose to open a separate PR to complete the ruff integration. They can probably go in parallel.