gh-129758: Correct imports in Lib/_pyrepl

[donbarbos]

• Issue: Inconsistent imports styles in Lib/_pyrepl #129758

[sobolevn]

I agree that changing if False to TYPE_CHECKING = False; if TYPE_CHECKING makes sense, because mypy better understands the second version.

See https://mypy-play.net/?mypy=latest&python=3.12&flags=strict%2Cwarn-unreachable&gist=ba9b71e677d20a0b4c1f78c731c0c0ea for if False:

main.py:4: error: Statement is unreachable  [unreachable]

And see https://mypy-play.net/?mypy=latest&python=3.12&flags=strict%2Cwarn-unreachable&gist=c84fabb19a5b4a92b6eee9656b978baa for if TYPE_CHECKING:

Succeeded!! (3383 ms)

So, basically - changing if False -> if TYPE_CHECKING is required to enable warn_unreachable = true.

But, I am against changing anything else. Let's no refactor other places, like moving imports or renaming things. It will just create more friction for no gain.

[bedevere-app]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[donbarbos]

it's probably worth leaving the removal of warnings in readline.py, it's redundant here because this file already has lazy imports of warnings.

I also removed import code and wrote from code import InteractiveConsole in if TYPE_CHECKING: so as not to import code (if this's not a typecheck runtime). And I removed from .readline import _setup like lazy import because we already have global imports of .readline.

And the last one I removed import ctypes because after a few lines all the objects that are used in the code are imported separately and they are also accessed, like:

import ctypes # so i removed this import
...
from ctypes import Structure
...
class CHAR_INFO(Structure):
...
class KeyEvent(ctypes.Structure): # and fix here: ctypes.Structure -> Structure
...

[donbarbos]

I have made the requested changes; please review again

[bedevere-app]

Thanks for making the requested changes!

@sobolevn: please review the changes made to this pull request.

[sobolevn]

I've left several suggestions to minimize the final diff. Every line counts 😉
We should be very careful about what we change and what we don't change.

Thank you!

[sobolevn]

Please, don't change this line.

[donbarbos]

@sobolevn ok, but why? We already have import _setup on 34 line

[AA-Turner]

In general, the default position is the status quo, you need to justify every change.

[donbarbos]

I removed it because it duplicates imports. As I already said it is on line 34

[AA-Turner]

I agree that changing if False to TYPE_CHECKING = False; if TYPE_CHECKING makes sense, because mypy better understands the second version.

@sobolevn is it feasible to add a config option to mypy to have it recognise if False as the same as if TYPE_CHECKING?

We have the same idiom in several places, and if it leaks further then we will have a lot of useless module-level constants, whereas if False is optimised away by the interpreter. I'd prefer to wait for mypy to release a new version with this new option, rather than to change code here needlessly (it also attracts a lot of low value contributions because "<tool> has warnings" etc).

A

[bedevere-app]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[donbarbos]

I have made the requested changes; please review again

[bedevere-app]

Thanks for making the requested changes!

@sobolevn, @AA-Turner: please review the changes made to this pull request.

[ambv]

Thanks, but we won't be taking this. This is churn.