Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Type hints in _type1font.py#30187
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
This file now passes `mypy --strict` (but its imports don't).Improve error handling: a new class _ParseError instead of ValueErroror RuntimeError. Raise most of the errors from utility functions tosimplify the parsing logic. Move FontBBox parsing into a new utilityfunction.
| if isinstance(self, _BinaryToken): | ||
| return self.raw[1:] | ||
| raise _ParseError(f"Token {self} is not a binary token") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I don't care too much about it, but the error/nonerror code flow in this function is reversed from the above one.
| def value(self) -> str | bytes: | ||
| return self._value() | ||
| @functools.lru_cache | ||
| defvalue(self): | ||
| def_value(self) -> str | bytes: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Doeslru_cache break something here? It seems the same as before, with an extra method.
| def __init__(self, input): | ||
| def __init__(self, input: str | tuple[bytes, bytes, bytes]): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The doc says already-decoded parts; is that wrong?
| @@ -377,7 +473,7 @@ def __init__(self, input): | |||
| self._abbr = {'RD': 'RD', 'ND': 'ND', 'NP': 'NP'} | |||
| self._parse() | |||
| def _read(self, file): | |||
| def _read(self, file: T.IO[bytes]) -> bytes: | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
That can be?
| def_read(self,file:T.IO[bytes])->bytes: | |
| def_read(self,file:T.BinaryIO)->bytes: |
| case int(): | ||
| key_int = key |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Coverage seems to think this line isn't tested; what needed this change?
| pos: dict[str, list[tuple[int, int]]] = {} | ||
| pos = {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
pos is set here twice.
| @@ -571,29 +705,20 @@ def _parse(self): | |||
| self._abbr['RD'] = key | |||
| # Fill in the various *Name properties | |||
| if'FontName'notinprop: | |||
| if not prop['FontName']: | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Can we be certain that an empty string is not allowed here (and below)?
| @@ -895,7 +1092,7 @@ def subset(self, characters, name_prefix): | |||
| )) | |||
| @staticmethod | |||
| def _charstring_tokens(data): | |||
| def _charstring_tokens(data: T.Iterable[int]) -> T.Generator[str | int, None, None]: | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
typing.Generator is deprecated.
| def_charstring_tokens(data:T.Iterable[int])->T.Generator[str|int,None,None]: | |
| def_charstring_tokens(data:T.Iterable[int])->collections.abc.Iterator[str|int]: |
| def _is_number(token: _Token[str] | _Token[bytes]) -> T.TypeGuard[_NumberToken]: | ||
| return token.is_number() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Unused?
| index_token = next(tokens) | ||
| if not index_token.is_number(): | ||
| _log.warning( | ||
| f"Parsing encoding: expected number, got {index_token}" | ||
| ) | ||
| continue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This previously ignored parsing failures; did you intend to raise here?
| T.cast(dict[str, T.Any], prop)[key], endpos = \ | ||
| subparsers[key](source, data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Maybe split these lines:
| T.cast(dict[str,T.Any],prop)[key],endpos= \ | |
| subparsers[key](source,data) | |
| value,endpos=subparsers[key](source,data) | |
| T.cast(dict[str,T.Any],prop)[key]=value |
| @@ -544,7 +678,7 @@ def _parse(self): | |||
| continue | |||
| if token.is_delim(): | |||
| value = _expression(token, source, data).raw | |||
| value: T.Any = _expression(token, source, data).raw | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This annotation seems unneeded?
This file now passes
mypy --strict(but its imports don't).Improve error handling: a new class _ParseError instead of ValueError
or RuntimeError. Raise most of the errors from utility functions to
simplify the parsing logic. Move FontBBox parsing into a new utility
function.