Type hints #3826

schriftgestalt · 2025-05-13T20:41:09Z

Add some type hints.

justvanrossum · 2025-05-13T20:52:07Z

Lib/fontTools/ttLib/tables/DefaultTable.py


    def __eq__(self, other):
-        if type(self) != type(other):
+        if not isinstance(self, type(other)):


This is unrelated to type hints and changes the meaning of the code. The current code is written as intended: only compare when the types match exactly.

You are right. I tried to avoid changes like this. I’ll remove it.

flake complains about the !=. Can this changed to is not?

justvanrossum · 2025-05-13T20:53:39Z

Lib/fontTools/ttLib/tables/otConverters.py

        assert writer.items[lengthIndex] == b"\xde\xad\xbe\xef"[: conv.staticSize]
        writer.items[lengthIndex] = lengthWriter.getAllData()

+    def __repr__(self):


I recomment to make a separate PR for changes other than type annotations.

schriftgestalt · 2025-05-13T21:52:23Z

I fixed a few flake issues I missed. But tox is not happy at all when I run it locally. Is there anything I can do or can I ignore issues in files I didn’t touch? I have fixes some easy ones. But that is probably another PR?

jenskutilek · 2025-05-15T11:42:15Z

I think you need to from __future__ import annotations for the tests to pass in Python 3.9.

And you should format the code with black to get rid of the linter fails.

jorenham

Here's a completely unsolicited review; I hope you don't mind :)

jorenham · 2025-10-04T00:32:06Z

Lib/fontTools/ttLib/tables/C_F_F_.py

 from io import BytesIO
 from fontTools import cffLib
 from . import DefaultTable
+from typing import Any, Dict, List, Optional, TYPE_CHECKING


There's no need for Dict, List anymore since Python 3.9 :)
https://peps.python.org/pep-0585/

jorenham · 2025-10-04T00:34:00Z

Lib/fontTools/ttLib/ttFont.py

-        _tableCache=None,
-        cfg={},
-    ):
+        file: str | IO[bytes] | Any | None = None,  # Using Any for generic file-like objects


Does os.PathLike (docs) fit the bill for "generic file-like objects"?

PathLike is for paths, like strings or Path objects. https://docs.python.org/3/library/typing.html#typing.IO is for File-like objects.

jorenham · 2025-10-04T00:38:14Z

Lib/fontTools/ttLib/ttFont.py

+        checkChecksums: int = 0,
+        verbose: bool | None = None,  # Deprecated
+        recalcBBoxes: bool = True,
+        allowVID: Any = NotImplemented,  # Deprecated/Unused


Once Python 3.9 support is no longer required, [types.NotImplementedType](https://docs.python.org/3/library/types.html#types.NotImplementedType) (docs) could be used for this. Otherwise, you could import it from typing_extensions from within a if TYPE_CHECKING: block (which magically doesn't require typing_extensions to be installed), and stringify-annotate it.

Hrm, I'd prefer a https://docs.python.org/3/library/exceptions.html#DeprecationWarning to be emitted if this parameter is used, rather than the type/default value being lied about.

jorenham · 2025-10-04T00:39:21Z

Lib/fontTools/ttLib/ttFont.py

+        recalcTimestamp: bool = True,
+        fontNumber: int = -1,
+        lazy: bool | None = None,
+        quiet: bool | None = None,  # Deprecated


If you want to avoid people from using this, perhaps it's an idea to annotate it as

Suggested change

quiet: bool | None = None, # Deprecated

quiet: None = None, # Deprecated

Hrm, I'd prefer a https://docs.python.org/3/library/exceptions.html#DeprecationWarning to be emitted if this parameter is used, rather than the type being lied about.

jorenham · 2025-10-04T00:47:29Z

Lib/fontTools/ttLib/ttFont.py

+        fontNumber: int = -1,
+        lazy: bool | None = None,
+        quiet: bool | None = None,  # Deprecated
+        _tableCache: dict[tuple[Tag, bytes], DefaultTable] | None = None,


Keep in mind that both type parameters of dict are invariant. So if you have

class DefaultTableSubclass(DefaultTable): ... my_table_cache: dict[tuple[Tag, bytes], DefaultTableSubclass] = ...

then you can't pass my_table_cache as _tableCache here. So the value types must be exactly the same.
This is also the case for the key type, so Tag subclasses won't work.

So if that's not what you want, then you could use collections.abc.Mapping, which has a covariant value type, but the key type is unfortunately also invariant (which is a known issue). To get around that, you could use a "free" type parameter, e.g. _TagT = TypeVar("_TagT", bound=Tag), and use that instead of Tag. Since it's not associated to anything else, it can materialize to anything assignable to Tag, effectively emulating covariant behavior ;)

Meaning that TTFont would need to use generics like so: class TTFont(Generic[_TagT])?

@anthrotype I don't think we ever subclass Tag, no? I see no reason to...

jorenham · 2025-10-04T00:58:06Z

Lib/fontTools/ttLib/ttFont.py

-    def __exit__(self, type, value, traceback):
+    def __exit__(
+        self,
+        exc_type: Type[BaseException] | None,


Suggested change

exc_type: Type[BaseException] | None,

exc_type: type[BaseException] | None,

jorenham · 2025-10-04T00:59:31Z

Lib/fontTools/ttLib/ttFont.py

-    def saveXML(self, fileOrPath, newlinestr="\n", **kwargs):
+    def saveXML(
+        self,
+        fileOrPath: str | IO[str] | Any,  # Use Any for generic text stream


maybe os.PathLike[str] could replace Any here?

Rather, os.PathLike[str] | IO[str] instead of str | IO[str] | Any.

str won't be assignable to that

Ugh. I forgot that PathLike is a special snowflake.

Well, I suppose there's _typeshed.StrPath you could use

jorenham · 2025-10-04T01:06:57Z

Lib/fontTools/ttLib/ttFont.py

+            report = "No '%s' table found." % tag
+            log.info(report)


I realize this is (also) out of scope, but the recommended way of formatting logging calls would in this case look something like this:

Suggested change

report = "No '%s' table found." % tag

log.info(report)

log.info( "No %r table found.", tag)

The %r effectively does a repr(_), which gives you free quotes.
And to be honest I can't remember exactly why the format arguments should be passed like this, just that there's a good reason for doing so haha.

I believe the string interpolation will only happen when the log happens, that's why they recommend doing it the way you suggest. https://docs.astral.sh/ruff/rules/logging-percent-format/

jorenham · 2025-10-04T01:09:25Z

Lib/fontTools/ttLib/ttFont.py

            del self.reader[tag]

-    def get(self, tag, default=None):
+    def get(self, tag: str, default: Any | None = None) -> DefaultTable | Any | None:


it might be nice to @overload this, and use a TypeVar for the default, similar to Mapping.get in https://github.com/python/typeshed/blob/aa52024657e7aecfe03d08a7e3719a2ded9a907a/stdlib/typing.pyi#L768-L773

jorenham · 2025-10-04T01:12:24Z

Lib/fontTools/ttLib/ttFont.py

-    ):
+        self,
+        preferCFF: bool = True,
+        location: dict[str, float] | None = None,


Note that this won't allow you to pass my_location: dict[str, int], but I'm not sure if that's such a bad thing here.

One could turn this into dict[str, float] | dict[str, int] | None. Not sure if there's a more elegant way.

You could use the dict[str, T] trick, with T = TypeVar("T", bound=float)

schriftgestalt · 2025-10-07T17:22:50Z

I currently have no time to work on this. Is there a way to work on this together?

[skip ci]

jorenham · 2025-10-08T14:49:28Z

I currently have no time to work on this. Is there a way to work on this together?

I wouldn't mind helping out. What do you propose?

madig · 2025-10-08T14:52:42Z

Lib/fontTools/ttLib/ttFont.py

+            str | IO[bytes] | Any | None
+        ) = None,  # Using Any for generic file-like objects
+        res_name_or_index: str | int | None = None,
+        sfntVersion: bytes = b"\000\001\000\000",


This is more correct, but there is code that checks for a str, not bytes...

schriftgestalt · 2025-10-08T14:56:16Z

there are a lot of good suggestions. Someone needs to put them in.
I’m not sure how that works git wise. Can you add commits to my fork?

madig · 2025-10-08T15:33:04Z

Lib/fontTools/ttLib/ttFont.py

+            if hasattr(current_file, "seekable"):
+                seekable = current_file.seekable()  # type: ignore
+            elif hasattr(current_file, "seek"):
                try:


What is the reason behind this change?

jorenham · 2025-10-08T15:35:45Z

there are a lot of good suggestions. Someone needs to put them in. I’m not sure how that works git wise. Can you add commits to my fork?

I'm not a maintainer here, so not directly. I could PR to your PR branch on your fork I suppose.

madig · 2025-10-08T15:36:03Z

I can push to the branch of your fork. I'm currently scratching my head a bit though, because there are various code changes that go way beyond adding type hints and break tests. I'm contemplating breaking off pieces of this PR and focussing on adding type hints without changing anything else.

anthrotype · 2025-10-08T15:38:36Z

focussing on adding type hints without changing anything else.

strong yes please. We already had a regression which slipped through another similar big type-hinting releated PR (for ufoLib) recently

Co-authored-by: schriftgestalt <georg.seifert@mac.com>

justvanrossum reviewed May 13, 2025

View reviewed changes

jorenham reviewed Oct 4, 2025

View reviewed changes

schriftgestalt added 3 commits October 8, 2025 15:31

cleanup

09e98c2

add typo hints

31f0320

add more specific __repr__

aea6f7a

madig force-pushed the TypeHints branch from 6ec855a to aea6f7a Compare October 8, 2025 14:33

Blacken

1041ca4

[skip ci]

madig marked this pull request as draft October 8, 2025 14:41

madig reviewed Oct 8, 2025

View reviewed changes

madig added a commit that referenced this pull request Oct 8, 2025

Break off typing part of #3826

a8bb38d

madig mentioned this pull request Oct 8, 2025

Break off typing part of #3826 #3952

Open

madig added a commit that referenced this pull request Oct 9, 2025

Break off typing part of #3826

18e2836

Co-authored-by: schriftgestalt <georg.seifert@mac.com>

madig added a commit that referenced this pull request Oct 9, 2025

Break off typing part of #3826

515fa76

Co-authored-by: schriftgestalt <georg.seifert@mac.com>

	quiet: bool \| None = None, # Deprecated
	quiet: None = None, # Deprecated

	exc_type: Type[BaseException] \| None,
	exc_type: type[BaseException] \| None,

	report = "No '%s' table found." % tag
	log.info(report)
	log.info( "No %r table found.", tag)

Type hints #3826

Are you sure you want to change the base?

Type hints #3826

Uh oh!

Conversation

schriftgestalt commented May 13, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

schriftgestalt commented May 13, 2025

Uh oh!

jenskutilek commented May 15, 2025

Uh oh!

jorenham left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

madig Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

schriftgestalt commented Oct 7, 2025

Uh oh!

jorenham commented Oct 8, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

schriftgestalt commented Oct 8, 2025

Uh oh!

madig Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jorenham commented Oct 8, 2025

Uh oh!

madig Oct 7, 2025 •

edited

Loading

madig Oct 8, 2025 •

edited

Loading