Skip to content

TYP: move imports inside TYPE_CHECKING #47968

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

Merged
merged 1 commit into from
Aug 4, 2022
Merged

TYP: move imports inside TYPE_CHECKING #47968

merged 1 commit into from
Aug 4, 2022

Conversation

twoertwein
Copy link
Member

These are found by flake8-type-checking. There are more cases (all of the _typing imports).

If flake8-type-checking should be run on the CI, it would be good to move all the _typing imports inside TYPE_CHECKING (not sure whether we want to do that).

@simonjayhawkins
Copy link
Member

There are more cases (all of the _typing imports).

This shouldn't be needed as we guard there so that there are no issues with importing from the top level. (and IIRC I rejected a PR that did that once before)

What problem are we trying to solve?

What you have done here though with the other imports lgtm.

@simonjayhawkins simonjayhawkins added the Typing type annotations, mypy/pyright type checking label Aug 4, 2022
@simonjayhawkins simonjayhawkins added this to the 1.5 milestone Aug 4, 2022
Copy link
Member

@simonjayhawkins simonjayhawkins left a comment

Choose a reason for hiding this comment

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

Thanks @twoertwein lgtm pending green

@twoertwein
Copy link
Member Author

What problem are we trying to solve?

The goal of flake8-type-checking is to reduce import times and cycling dependencies by moving as much into TYPE_CHECKING. Having it run on the CI would ensure that imports that could be in the TYPE_CHECKING block are there. Probably not worth running it on the CI, would need too many ignores for each line of a _typing import.

I purposefully avoided the _typing imports as I also remember that there were some discussions about not doing that some time ago :)

@simonjayhawkins
Copy link
Member

The goal of flake8-type-checking is to reduce import times

now that's a fair point that I don't recall discussing before, so potentially open to the idea.

@simonjayhawkins simonjayhawkins merged commit 7214f85 into pandas-dev:main Aug 4, 2022
@simonjayhawkins
Copy link
Member

Thanks @twoertwein

@twoertwein twoertwein deleted the imports branch September 10, 2022 01:38
noatamir pushed a commit to noatamir/pandas that referenced this pull request Nov 9, 2022
@twoertwein twoertwein mentioned this pull request Feb 28, 2023
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Typing type annotations, mypy/pyright type checking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants