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

Start adding types to Submodule, add py.typed to manifest #1282

Merged
merged 12 commits into from
Jul 1, 2021
Merged

Start adding types to Submodule, add py.typed to manifest #1282

merged 12 commits into from
Jul 1, 2021

Conversation

Yobmod
Copy link
Contributor

@Yobmod Yobmod commented Jun 30, 2021

Started to add types to Submodule

Removed include_package_data() from setup and added py.typed to MANIFEST.INI instead (this is what mypy package does, but not what the mypy instructions say!)

Improve types for .traverse() with TypeVar and overloads, create TraversableIterableObj class to reduce code duplication from overloads.

Add some more checks to test_commit.test_traverse()

Copy link
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's great work, thanks a lot!

It's good to see that people are already starting to use it, too.

Can't wait to merge.

git/objects/submodule/base.py Outdated Show resolved Hide resolved
setup.py Show resolved Hide resolved
@Byron Byron merged commit 18c777b into gitpython-developers:main Jul 1, 2021
@Byron
Copy link
Member

Byron commented Jul 2, 2021

Thanks a million for all your work!
It's clear you are committed to getting this done and I would be glad if you would want to become a maintainer. Now that types have been added and it feels like the project would be in serious trouble without your continued support in keeping maintaining types as python evolves along with the type checkers.
Thanks a lot for your consideration.

@Byron Byron added this to the v3.1.19 - Bugfixes milestone Jul 23, 2021
EliahKagan added a commit to EliahKagan/GitPython that referenced this pull request Dec 22, 2023
Returning an explicit value from a generator function causes that
value to be bound to the `value` attribute of the StopIteration
exception. This is available as the result of "yield from" when it
is used as an expression; or by explicitly catching StopIteration,
binding the StopIteration exception to a variable, and accessing
the attribute. This feature of generators is rarely used.

The `return iter([])` statement in Submodule.iter_items uses this
feature, causing the resulting StopIteration exception object to
have a `value` attribute that refers to a separate second iterator
that also yields no values. From context, this behavior is clearly
not the goal; a bare return statement should be used here (which
has the same effect except for the `value` attribute of the
StopIteration exception). The code had used a bare return prior to
82b131c (gitpython-developers#1282), when `return` was changed to `return iter([])`.
That was part of a change that added numerous type annotations. It
looks like it was either a mistake, or possibly an attempt to work
around an old bug in a static type checker.

This commit extends the test_iter_items_from_invalid_hash test to
assert that the `value` attribute of the StopIteration is its usual
default value of None. This commit only extends the test; it does
not fix the bug.
EliahKagan added a commit to EliahKagan/GitPython that referenced this pull request Dec 22, 2023
Returning an explicit value from a generator function causes that
value to be bound to the `value` attribute of the StopIteration
exception. This is available as the result of "yield from" when it
is used as an expression; or by explicitly catching StopIteration,
binding the StopIteration exception to a variable, and accessing
the attribute. This feature of generators is rarely used.

The `return iter([])` statement in Submodule.iter_items uses this
feature, causing the resulting StopIteration exception object to
have a `value` attribute that refers to a separate second iterator
that also yields no values. From context, this behavior is clearly
not the goal; a bare return statement should be used here (which
has the same effect except for the `value` attribute of the
StopIteration exception). The code had used a bare return prior to
82b131c (gitpython-developers#1282), when `return` was changed to `return iter([])`.
That was part of a change that added numerous type annotations. It
looks like it was either a mistake, or possibly an attempt to work
around an old bug in a static type checker.

This commit extends the test_iter_items_from_invalid_hash test to
assert that the `value` attribute of the StopIteration is its usual
default value of None. This commit only extends the test; it does
not fix the bug.
EliahKagan added a commit to EliahKagan/GitPython that referenced this pull request Dec 22, 2023
Returning an explicit value from a generator function causes that
value to be bound to the `value` attribute of the StopIteration
exception. This is available as the result of "yield from" when it
is used as an expression; or by explicitly catching StopIteration,
binding the StopIteration exception to a variable, and accessing
the attribute. This feature of generators is rarely used.

The `return iter([])` statement in Submodule.iter_items uses this
feature, causing the resulting StopIteration exception object to
have a `value` attribute that refers to a separate second iterator
that also yields no values (gitpython-developers#1779).

From context, this behavior is clearly not the goal; a bare return
statement should be used here (which has the same effect except for
the `value` attribute of the StopIteration exception). The code had
used a bare return prior to 82b131c (gitpython-developers#1282), when `return` was
changed to `return iter([])`. That was part of a change that added
numerous type annotations. It looks like it was either a mistake,
or possibly an attempt to work around an old bug in a static type
checker.

This commit extends the test_iter_items_from_invalid_hash test to
assert that the `value` attribute of the StopIteration is its usual
default value of None. This commit only extends the test; it does
not fix the bug.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants