Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type hints for _pygit2 (C module) #1121

Merged
merged 8 commits into from
Mar 6, 2022
Merged

Type hints for _pygit2 (C module) #1121

merged 8 commits into from
Mar 6, 2022

Conversation

jorio
Copy link
Contributor

@jorio jorio commented Jan 30, 2022

In IDEs like PyCharm, type hinting doesn't work well with the contents of pygit2._pygit2 (the part of pygit2 that's written in C). This hinders autocompletion for important classes like Commit, Branch, etc.

This PR introduces _pygit2.pyi, which annotates types implemented in C. This makes it much more pleasant to work with pygit2 in an IDE.

In a nutshell, I used stubgen to create type stubs. Then I refined them manually, and I ensured that stubtest doesn't report errors.

If we merge this PR, we should keep the stubs file in sync with any API changes we make in the C codebase from now on. The consistency of the type stubs can be tested with build.sh stubtest — I suggest we add this step to CI. Stubtest isn't perfect: it won't catch discrepancies in argument types, but it will raise errors if we miss a new function or member variable.


This PR ties in with issue #709, but it doesn't fix it completely: we'll still need to annotate more places in the Python codebase.

@jdavid
Copy link
Member

jdavid commented Feb 1, 2022

This is great!

Just sometimes we loss some information, for example:

-  "applies(diff, location=GIT_APPLY_LOCATION_INDEX) -> bool\n"
+  "applies(diff: Diff, location: int = ...) -> bool\n"

Here we win the type information, in the docs for example this renders a nice link for Diff. But we lose the default value for the location argument. Cannot we keep both?

@jorio
Copy link
Contributor Author

jorio commented Feb 1, 2022

Fixed! That was an oversight.

@jdavid
Copy link
Member

jdavid commented Feb 7, 2022

For discover_repository today the docs are (see https://www.pygit2.org/repository.html#pygit2.discover_repository):

pygit2.discover_repository(path[, across_fs[, ceiling_dirs]]) → str

With this PR it will read:

pygit2.discover_repository(path: str, across_fs: bool = False, ceiling_dirs: str = ...) → str

The ellipsis default value for ceiling_dirs is kind of undefined. In this particular case the default value should be the empty string; actually omitting ceiling_dirs will pass NULL to libgit2, but in this case (git_repository_discover) it's equivalent to the empty string.

From what I've seen the documentation is generated from the doc strings, while the editors use the information in the type hints?

So both have to be kept synchronized. For example Worktree.prune(force=False) is correctly rendered in the documentation, even though in the stubs file it uses the ellipsis.

Then, can you please:

  • Check that the documentation strings and the type hints match
  • Replace the ellipsis by a proper default value in function arguments

Thanks!

@jorio
Copy link
Contributor Author

jorio commented Mar 5, 2022

Some ellipses were oversights (such as Worktree.prune), but most were deliberate, because many pygit2 functions implemented in C don't have fallback values for optional parameters. For instance:

Tree.diff_to_tree([tree: Tree, flags: int, context_lines: int, interhunk_lines: int, swap: bool = False])

When calling this function, it's OK to omit tree. However, passing tree=None causes TypeError, which isn't the same effect as omitting the argument. This is because the C implementation doesn't check for None; it only considers whether the parameter is present or not.

This construct (optional arguments without a default value) is difficult to translate to the type stub syntax. It'd be misleading to have tree=None, and the [] syntax doesn't exist in type stubs, which is why I went for tree=....

That said, I understand your concern about losing information in the docs, so my latest commit in the PR restores the old square-bracket notation in the PyDoc_STRVARs wherever there is no adequate default value.

The type stubs still have some ellipses out of necessity, but this is no problem for the HTML docs because Sphinx only looks at the C code, not at the stubs.

@jdavid jdavid merged commit 2d09761 into libgit2:master Mar 6, 2022
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Sep 6, 2022
1.10.1 (2022-08-28)
-------------------------

- Fix segfault in ``Signature`` repr
  `#1155 <https://github.com/libgit2/pygit2/pull/1155>`_

- Linux and macOS wheels for Python 3.11
  `#1154 <https://github.com/libgit2/pygit2/pull/1154>`_


1.10.0 (2022-07-24)
-------------------------

- Upgrade to libgit2 1.5

- Add support for ``GIT_OPT_GET_OWNER_VALIDATION`` and
  ``GIT_OPT_SET_OWNER_VALIDATION``
  `#1150 <https://github.com/libgit2/pygit2/pull/1150>`_

- New ``untracked_files`` and ``ignored`` optional arguments for
  ``Repository.status(...)``
  `#1151 <https://github.com/libgit2/pygit2/pull/1151>`_


1.9.2 (2022-05-24)
-------------------------

- New ``Repository.create_commit_string(...)`` and
  ``Repository.create_commit_with_signature(...)``
  `#1142 <https://github.com/libgit2/pygit2/pull/1142>`_

- Linux and macOS wheels updated to libgit2 v1.4.3

- Remove redundant line
  `#1139 <https://github.com/libgit2/pygit2/pull/1139>`_


1.9.1 (2022-03-22)
-------------------------

- Type hints: added to C code and Branches/References
  `#1121 <https://github.com/libgit2/pygit2/pull/1121>`_
  `#1132 <https://github.com/libgit2/pygit2/pull/1132>`_

- New ``Signature`` supports ``str()`` and ``repr()``
  `#1135 <https://github.com/libgit2/pygit2/pull/1135>`_

- Fix ODB backend's read in big endian architectures
  `#1130 <https://github.com/libgit2/pygit2/pull/1130>`_

- Fix install with poetry
  `#1129 <https://github.com/libgit2/pygit2/pull/1129>`_
  `#1128 <https://github.com/libgit2/pygit2/issues/1128>`_

- Wheels: update to libgit2 v1.4.2

- Tests: fix testing ``parse_diff``
  `#1131 <https://github.com/libgit2/pygit2/pull/1131>`_

- CI: various fixes after migration to libgit2 v1.4


1.9.0 (2022-02-22)
-------------------------

- Upgrade to libgit2 v1.4

- Documentation, new recipes for committing and cloning
  `#1125 <https://github.com/libgit2/pygit2/pull/1125>`_


1.8.0 (2022-02-04)
-------------------------

- Rename ``RemoteCallbacks.progress(...)`` callback to ``.sideband_progress(...)``
  `#1120 <https://github.com/libgit2/pygit2/pull/1120>`_

- New ``Repository.merge_base_many(...)`` and ``Repository.merge_base_octopus(...)``
  `#1112 <https://github.com/libgit2/pygit2/pull/1112>`_

- New ``Repository.listall_stashes()``
  `#1117 <https://github.com/libgit2/pygit2/pull/1117>`_

- Code cleanup
  `#1118 <https://github.com/libgit2/pygit2/pull/1118>`_

Backward incompatible changes:

- The ``RemoteCallbacks.progress(...)`` callback has been renamed to
  ``RemoteCallbacks.sideband_progress(...)``. This matches the documentation,
  but may break existing code that still uses the old name.


1.7.2 (2021-12-06)
-------------------------

- Universal wheels for macOS
  `#1109 <https://github.com/libgit2/pygit2/pull/1109>`_


1.7.1 (2021-11-19)
-------------------------

- New ``Repository.amend_commit(...)``
  `#1098 <https://github.com/libgit2/pygit2/pull/1098>`_

- New ``Commit.message_trailers``
  `#1101 <https://github.com/libgit2/pygit2/pull/1101>`_

- Windows wheels for Python 3.10
  `#1103 <https://github.com/libgit2/pygit2/pull/1103>`_

- Changed: now ``DiffDelta.is_binary`` returns ``None`` if the file data has
  not yet been loaded, cf. `#962 <https://github.com/libgit2/pygit2/issues/962>`_

- Document ``Repository.get_attr(...)`` and update theme
  `#1017 <https://github.com/libgit2/pygit2/issues/1017>`_
  `#1105 <https://github.com/libgit2/pygit2/pull/1105>`_


1.7.0 (2021-10-08)
-------------------------

- Upgrade to libgit2 1.3.0
  `#1089 <https://github.com/libgit2/pygit2/pull/1089>`_

- Linux wheels now bundled with libssh2 1.10.0 (instead of 1.9.0)

- macOS wheels now include libssh2

- Add support for Python 3.10
  `#1092 <https://github.com/libgit2/pygit2/pull/1092>`_
  `#1093 <https://github.com/libgit2/pygit2/pull/1093>`_

- Drop support for Python 3.6

- New `pygit2.GIT_CHECKOUT_SKIP_LOCKED_DIRECTORIES`
  `#1087 <https://github.com/libgit2/pygit2/pull/1087>`_

- New optional argument ``location`` in ``Repository.applies(..)`` and
  ``Repository.apply(..)``
  `#1091 <https://github.com/libgit2/pygit2/pull/1091>`_

- Fix: Now the `flags` argument in `Repository.blame()` is passed through
  `#1083 <https://github.com/libgit2/pygit2/pull/1083>`_

- CI: Stop using Travis, move to GitHub actions

Caveats:

- Windows wheels for Python 3.10 not yet available.
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Sep 6, 2022
1.10.1 (2022-08-28)
-------------------------

- Fix segfault in ``Signature`` repr
  `#1155 <https://github.com/libgit2/pygit2/pull/1155>`_

- Linux and macOS wheels for Python 3.11
  `#1154 <https://github.com/libgit2/pygit2/pull/1154>`_


1.10.0 (2022-07-24)
-------------------------

- Upgrade to libgit2 1.5

- Add support for ``GIT_OPT_GET_OWNER_VALIDATION`` and
  ``GIT_OPT_SET_OWNER_VALIDATION``
  `#1150 <https://github.com/libgit2/pygit2/pull/1150>`_

- New ``untracked_files`` and ``ignored`` optional arguments for
  ``Repository.status(...)``
  `#1151 <https://github.com/libgit2/pygit2/pull/1151>`_


1.9.2 (2022-05-24)
-------------------------

- New ``Repository.create_commit_string(...)`` and
  ``Repository.create_commit_with_signature(...)``
  `#1142 <https://github.com/libgit2/pygit2/pull/1142>`_

- Linux and macOS wheels updated to libgit2 v1.4.3

- Remove redundant line
  `#1139 <https://github.com/libgit2/pygit2/pull/1139>`_


1.9.1 (2022-03-22)
-------------------------

- Type hints: added to C code and Branches/References
  `#1121 <https://github.com/libgit2/pygit2/pull/1121>`_
  `#1132 <https://github.com/libgit2/pygit2/pull/1132>`_

- New ``Signature`` supports ``str()`` and ``repr()``
  `#1135 <https://github.com/libgit2/pygit2/pull/1135>`_

- Fix ODB backend's read in big endian architectures
  `#1130 <https://github.com/libgit2/pygit2/pull/1130>`_

- Fix install with poetry
  `#1129 <https://github.com/libgit2/pygit2/pull/1129>`_
  `#1128 <https://github.com/libgit2/pygit2/issues/1128>`_

- Wheels: update to libgit2 v1.4.2

- Tests: fix testing ``parse_diff``
  `#1131 <https://github.com/libgit2/pygit2/pull/1131>`_

- CI: various fixes after migration to libgit2 v1.4


1.9.0 (2022-02-22)
-------------------------

- Upgrade to libgit2 v1.4

- Documentation, new recipes for committing and cloning
  `#1125 <https://github.com/libgit2/pygit2/pull/1125>`_


1.8.0 (2022-02-04)
-------------------------

- Rename ``RemoteCallbacks.progress(...)`` callback to ``.sideband_progress(...)``
  `#1120 <https://github.com/libgit2/pygit2/pull/1120>`_

- New ``Repository.merge_base_many(...)`` and ``Repository.merge_base_octopus(...)``
  `#1112 <https://github.com/libgit2/pygit2/pull/1112>`_

- New ``Repository.listall_stashes()``
  `#1117 <https://github.com/libgit2/pygit2/pull/1117>`_

- Code cleanup
  `#1118 <https://github.com/libgit2/pygit2/pull/1118>`_

Backward incompatible changes:

- The ``RemoteCallbacks.progress(...)`` callback has been renamed to
  ``RemoteCallbacks.sideband_progress(...)``. This matches the documentation,
  but may break existing code that still uses the old name.


1.7.2 (2021-12-06)
-------------------------

- Universal wheels for macOS
  `#1109 <https://github.com/libgit2/pygit2/pull/1109>`_


1.7.1 (2021-11-19)
-------------------------

- New ``Repository.amend_commit(...)``
  `#1098 <https://github.com/libgit2/pygit2/pull/1098>`_

- New ``Commit.message_trailers``
  `#1101 <https://github.com/libgit2/pygit2/pull/1101>`_

- Windows wheels for Python 3.10
  `#1103 <https://github.com/libgit2/pygit2/pull/1103>`_

- Changed: now ``DiffDelta.is_binary`` returns ``None`` if the file data has
  not yet been loaded, cf. `#962 <https://github.com/libgit2/pygit2/issues/962>`_

- Document ``Repository.get_attr(...)`` and update theme
  `#1017 <https://github.com/libgit2/pygit2/issues/1017>`_
  `#1105 <https://github.com/libgit2/pygit2/pull/1105>`_


1.7.0 (2021-10-08)
-------------------------

- Upgrade to libgit2 1.3.0
  `#1089 <https://github.com/libgit2/pygit2/pull/1089>`_

- Linux wheels now bundled with libssh2 1.10.0 (instead of 1.9.0)

- macOS wheels now include libssh2

- Add support for Python 3.10
  `#1092 <https://github.com/libgit2/pygit2/pull/1092>`_
  `#1093 <https://github.com/libgit2/pygit2/pull/1093>`_

- Drop support for Python 3.6

- New `pygit2.GIT_CHECKOUT_SKIP_LOCKED_DIRECTORIES`
  `#1087 <https://github.com/libgit2/pygit2/pull/1087>`_

- New optional argument ``location`` in ``Repository.applies(..)`` and
  ``Repository.apply(..)``
  `#1091 <https://github.com/libgit2/pygit2/pull/1091>`_

- Fix: Now the `flags` argument in `Repository.blame()` is passed through
  `#1083 <https://github.com/libgit2/pygit2/pull/1083>`_

- CI: Stop using Travis, move to GitHub actions

Caveats:

- Windows wheels for Python 3.10 not yet available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants