Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

pelson
Copy link
Contributor

@pelson pelson commented Oct 15, 2025

Improve the typing of the with_cleanup decorator, as used by the download, install, lock, and wheel commands.

There is zero functional change in this PR, and the typing usage is functional back to Python 3.7 (at least), so should be no compatibility issues.

Originally identified as a result of #13599.

self: _CommandT,
options: Values,
args: list[str],
) -> int:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that there is no use of the | None, and in all cases the args is list[str]:

@sepehr-rs
Copy link
Member

Hi @pelson, thanks a lot for contributing to pip!
This PR looks mostly internal, so I think it doesn’t need a news item. Please note that it might take a bit of time for a maintainer to review, since the team works on a volunteer basis. Thanks for your patience!

@sepehr-rs sepehr-rs added the skip news Does not need a NEWS file entry (eg: trivial changes) label Oct 15, 2025
)


_CommandT = TypeVar("_CommandT", bound=RequirementCommand)
Copy link
Member

Choose a reason for hiding this comment

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

Aren't TypeVars usually defined shortly before they're first used or shortly after the imports or even in the if TYPE_CHECKING block?

Regardless, please move it somewhere other than the very end of the module.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mechanically this is done because with_cleanup depends on RequirementCommand, but that is defined afterwards, so I was forced to make this typevar here.

I am fine to move the with_cleanup definition to after RequirementCommand if this is what you'd like me to do.

Copy link
Member

Choose a reason for hiding this comment

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

IIRC mypy will tolerate a forward reference in a typevar bound just fine.

from typing import TypeVar

T = TypeVar("T", bound="Base")

def do_something(obj: T) -> T:
	return obj

class Base:
	pass

class Wrong:
	pass

do_something(Base())   # passes
do_something(Wrong())  # E: Value of type variable "T" of "do_something" cannot be "Wrong"  [type-var]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Confirmed. Updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip news Does not need a NEWS file entry (eg: trivial changes)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.