Add stubs for pygit2 by xen0n · Pull Request #11374 · python/typeshed

xen0n · 2024-02-07T10:16:50Z

The upstream library is very tricky to type (likely requires nontrivial refactoring), and only contains partial type information, but stubs are a lot easier because only the public signatures are involved this way, so I plan to first make the library usable in typed projects by making stubs available here, then gradually work my way upstream.

The stubs are auto-generated then completed with fully manual inspection of every Python source file. The _pygit2.pyi comes from upstream and is mostly untouched except for required style changes, the signature of options(), and FilterSource which is missing from upstream.

JelleZijlstra

Thank you. I looked through the code up to config.pyi, lightly comparing the stubs against the implementation. Mostly it looks good, but I see quite a few incorrect uses of AnyStr_co.

JelleZijlstra · 2024-02-17T04:17:58Z

+
+from . import enums
+from ._build import __version__ as __version__
+from ._pygit2 import (


The runtime just does from ._pygit2 import *, why not do that here?

Originally I kept the star imports, but then mypy started to complain about wrong types of settings (it seems unable to distinguish the module settings and the global variable settings for example), and stubtest also wasn't happy with all those re-exported names either, so I had to spell out the names to basically help the checkers properly resolve the names.

JelleZijlstra · 2024-02-17T04:18:36Z

+    get_credentials as get_credentials,
+)
+from .config import Config as Config
+from .credentials import (


This one is also import *ed.

JelleZijlstra · 2024-02-17T04:18:40Z

+from .errors import Passthrough as Passthrough
+from .filter import Filter as Filter
+from .index import Index as Index, IndexEntry as IndexEntry
+from .legacyenums import (


And this one

JelleZijlstra · 2024-02-17T04:19:52Z

+LIBGIT2_VER: tuple[int, int, int]
+
+def init_repository(
+    path: str | bytes | PathLike[AnyStr_co] | None,


A TypeVar doesn't make sense here since there is only one occurrence in the function. In any case, covariance is meaningless in a function.

You can instead use the StrOrBytesPath from _typeshed.

Thanks for the suggestion; I only recently returned to Python development so I'm still getting familiar with the new typing scheme. I'll push an updated version later.

JelleZijlstra · 2024-02-17T04:21:19Z

+
+__version__: str
+
+def get_libgit2_paths() -> tuple[Path, dict[str, Any]]: ...


Suggested change

def get_libgit2_paths() -> tuple[Path, dict[str, Any]]: ...

def get_libgit2_paths() -> tuple[Path, dict[str, list[str]]]: ...

JelleZijlstra · 2024-02-17T04:32:42Z

+from .utils import _IntoStrArray
+
+class Payload:
+    def __init__(self, **kw: dict[str, object]) -> None: ...


Suggested change

def __init__(self, **kw: dict[str, object]) -> None: ...

def __init__(self, **kw: object) -> None: ...

Note that the type for **kwargs is the type of individual kwargs, not of the whole kwargs dict.

JelleZijlstra · 2024-02-17T04:36:30Z

+    directory: PathLike[AnyStr_co] | bytes | str | None = None,
+    paths: _IntoStrArray[AnyStr_co] = None,


It doesn't look like this function really needs to be generic. These types imply that directory and paths must be of the same type, which doesn't seem to be the case.

JelleZijlstra · 2024-02-17T04:36:38Z

+    callbacks: StashApplyCallbacks | None = None,
+    reinstate_index: bool = False,
+    strategy: CheckoutStrategy | None = None,
+    directory: PathLike[AnyStr_co] | bytes | str | None = None,


JelleZijlstra · 2024-02-17T04:37:14Z

+
+from _cffi_backend import _CDataBase
+
+def str_to_bytes(value: object, name: str) -> bytes: ...


Suggested change

def str_to_bytes(value: object, name: str) -> bytes: ...

def str_to_bytes(value: str, name: object) -> bytes: ...

It throws a TypeError if it's not a str

That must be a thinko. Fixed in the next push.

JelleZijlstra · 2024-02-17T04:38:08Z

+    def __del__(self) -> None: ...
+    def __contains__(self, key: str) -> bool: ...
+    def __getitem__(self, key: str) -> str: ...
+    def __setitem__(self, key: str, value: bool | int | _CDataBase | PathLike[AnyStr_co] | bytes | str | None) -> None: ...


Again, don't think AnyStr_co makes sense here.

xen0n · 2024-02-28T12:06:44Z

I've just removed all AnyStr_co occurrences and fixed the rest of signature mistakes pointed out in the review comments.

xen0n · 2024-02-28T13:36:06Z

It seems mypy cannot handle the re-import of Repository with different types if the star-imports have to be preserved in the stubs:

stubs/pygit2/pygit2/__init__.pyi:24: error: Incompatible import of "Repository" (imported name has type "type[pygit2.repository.Repository]", local name has type "type[pygit2._pygit2.Repository]")  [assignment]

This is the same as python/typeshed#11374, and allows proper type-checking before it is accepted upstream.

The upstream library is very tricky to type (likely requires nontrivial refactoring), and only contains partial type information, but stubs are a lot easier because only the public signatures are involved this way, so I plan to first make the library usable in typed projects by making stubs available here, then gradually work my way upstream. The stubs are auto-generated then completed with fully manual inspection of every Python source file. The `_pygit2.pyi` comes from upstream and is mostly untouched except for required style changes, the signature of `options()`, and `FilterSource` which is missing from upstream.

github-actions · 2024-03-02T08:06:21Z

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

xen0n · 2024-03-02T08:49:16Z

It seems simply ignoring the type check error on the Repository re-import doesn't work. When I was working on a project with the content of this PR integrated into its stubs/ I see Pylance failing to infer type for pygit2.Repository (saying the name doesn't exist).

This is the same as python/typeshed#11374, and allows proper type-checking before it is accepted upstream.

This comment has been minimized.

Sign in to view

xen0n force-pushed the pygit2 branch 4 times, most recently from 9e7fd82 to 7f3e7da Compare February 7, 2024 10:46

This comment has been minimized.

Sign in to view

xen0n force-pushed the pygit2 branch from 7f3e7da to 084a06b Compare February 7, 2024 10:58

This comment has been minimized.

Sign in to view

xen0n force-pushed the pygit2 branch from 084a06b to e4c2f3a Compare February 7, 2024 11:10

This comment has been minimized.

Sign in to view

xen0n force-pushed the pygit2 branch 2 times, most recently from 351dd7e to 37f496a Compare February 7, 2024 11:36

This comment has been minimized.

Sign in to view

xen0n force-pushed the pygit2 branch 2 times, most recently from 76491de to 87d8532 Compare February 7, 2024 11:51

xen0n mentioned this pull request Feb 7, 2024

Migrate from GitPython to pygit2 ruyisdk/ruyi#54

Closed

This comment has been minimized.

Sign in to view

xen0n force-pushed the pygit2 branch from 87d8532 to a1e4ab8 Compare February 8, 2024 09:53

This comment has been minimized.

Sign in to view

xen0n force-pushed the pygit2 branch from a1e4ab8 to 8a3b924 Compare February 8, 2024 10:04

This comment has been minimized.

Sign in to view

JelleZijlstra reviewed Feb 17, 2024

View reviewed changes

xen0n force-pushed the pygit2 branch from 8a3b924 to 1cb8da7 Compare February 28, 2024 11:51

This comment has been minimized.

Sign in to view

xen0n force-pushed the pygit2 branch from eea4b4e to a0f57b8 Compare February 28, 2024 12:09

This comment has been minimized.

Sign in to view

xen0n force-pushed the pygit2 branch from a0f57b8 to 7352a09 Compare February 28, 2024 13:28

This comment has been minimized.

Sign in to view

xen0n added a commit to xen0n/ruyi that referenced this pull request Feb 28, 2024

build: add type stubs for pygit2

d5891f0

This is the same as python/typeshed#11374, and allows proper type-checking before it is accepted upstream.

xen0n mentioned this pull request Mar 1, 2024

Imports that shadow names cannot be made to type-check python/mypy#16972

Open

xen0n added 3 commits March 2, 2024 15:34

pygit2: Fix some oversights

dd64b26

pygit2: Annotate the undocumented _cache_enums in _pygit2.pyi

647cb2f

xen0n force-pushed the pygit2 branch from 7352a09 to 46951e5 Compare March 2, 2024 07:34

This comment has been minimized.

Sign in to view

xen0n added 7 commits March 2, 2024 15:59

pygit2: Eliminate wrong usage of AnyStr_co

80b2d81

pygit2: Fix signatures per review

2d741c9

pygit2: Sync with v1.14.1 changes

c01a154

pygit2: Add several missing annotations

9c7367d

pygit2: Add missing annotations for methods implemented in C

ce9b830

pygit2: Workaround the false-positive mypy error on shadowing import

09938ef

pygit2: Update stubtest allowlist for false-positives

5c7236f

xen0n force-pushed the pygit2 branch from c6992c7 to 5c7236f Compare March 2, 2024 07:59

xen0n mentioned this pull request Mar 4, 2024

Format the repository and enforce code style in CI libgit2/pygit2#1280

Merged

xen0n added a commit to xen0n/ruyi that referenced this pull request Mar 4, 2024

build: add type stubs for pygit2

95dcb8b

This is the same as python/typeshed#11374, and allows proper type-checking before it is accepted upstream.

JelleZijlstra approved these changes Mar 12, 2024

View reviewed changes

JelleZijlstra merged commit 9a4b605 into python:main Mar 12, 2024

xen0n deleted the pygit2 branch March 12, 2024 07:28


		__version__: str

		def get_libgit2_paths() -> tuple[Path, dict[str, Any]]: ...

	def __init__(self, **kw: dict[str, object]) -> None: ...
	def __init__(self, **kw: object) -> None: ...

		directory: PathLike[AnyStr_co] \| bytes \| str \| None = None,
		paths: _IntoStrArray[AnyStr_co] = None,


		from _cffi_backend import _CDataBase

		def str_to_bytes(value: object, name: str) -> bytes: ...

	def str_to_bytes(value: object, name: str) -> bytes: ...
	def str_to_bytes(value: str, name: object) -> bytes: ...

Uh oh!

Uh oh!

Conversation

xen0n commented Feb 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

JelleZijlstra 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

xen0n commented Feb 28, 2024

Uh oh!

This comment has been minimized.

This comment has been minimized.

xen0n commented Feb 28, 2024

Uh oh!

This comment has been minimized.

github-actions Bot commented Mar 2, 2024

Uh oh!

xen0n commented Mar 2, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

xen0n commented Feb 7, 2024 •

edited

Loading