FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

FileFinder takes a class, not an instance (#7085) by Spindel · Pull Request #7086 · python/typeshed · GitHub

  1. python
  2. typeshed

Repository navigation

FileFinder takes a class, not an instance (#7085) - #7086

Merged
JelleZijlstra merged 1 commit into
python:masterfrom
Spindel:stdlib-importlib-machinery-loader
Jan 30, 2022
ConversationCommits (1)ChecksFiles changed
Merged

FileFinder takes a class, not an instance (#7085)#7086
JelleZijlstra merged 1 commit into
python:masterfrom
Spindel:stdlib-importlib-machinery-loader

Conversation

Spindel commented Jan 30, 2022

Copy link
Copy Markdown
Contributor

The FileFinder takes a tuple of (class, arguments) to instantiate for
each time, rather than an existing instance.

This fixes the type hint to match Python 3.10 reality.

Issue: #7085

This comment has been minimized.

Sign in to view
AlexWaygood reviewed Jan 30, 2022
View reviewed changes

AlexWaygood left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

from __future__ import annotations is implicitly enabled in stub files, so we can just use builtin type instead of typing.Type — this is what the flake8 error message was trying to flag.

(Do you have any feedback for how we could improve that flake8 error message, btw? 🙂)

Comment thread stdlib/importlib/machinery.pyi Outdated
Comment thread stdlib/importlib/machinery.pyi Outdated

This comment has been minimized.

Sign in to view
AlexWaygood reviewed Jan 30, 2022
View reviewed changes
Comment thread stdlib/importlib/machinery.pyi Outdated

This comment has been minimized.

Sign in to view
AlexWaygood mentioned this pull request Jan 30, 2022
Closed

AlexWaygood commented Jan 30, 2022

Copy link
Copy Markdown
Member

I have opened PyCQA/flake8-pyi#164 — we can definitely improve that error message :)

As the person who, er, wrote the error message, I sincerely apologise for the confusion!!

This comment has been minimized.

Sign in to view
FileFinder takes a class, not an instance (#7085)
a376fcd 
The FileFinder takes a tuple of (class, arguments) to instantiate for
each time, rather than an existing instance.

This fixes the type hint to match Python 3.10 reality.

Issue: #7085

Spindel commented Jan 30, 2022

Copy link
Copy Markdown
Contributor Author

(Do you have any feedback for how we could improve that flake8 error message, btw? slightly_smiling_face)

Perhaps Y022 It is preferred to use type[C] over typing.Type[C]

Spindel commented Jan 30, 2022

Copy link
Copy Markdown
Contributor Author

Anyhow, I've squashed away the evidence of my confusion, and it should pass the lints now, thanks for your help!

AlexWaygood commented Jan 30, 2022

Copy link
Copy Markdown
Member

Anyhow, I've squashed away the evidence of my confusion, and it should pass the lints now, thanks for your help!

No problem!!

github-actions Bot commented Jan 30, 2022

Copy link
Copy Markdown
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

Spindel commented Jan 30, 2022

Copy link
Copy Markdown
Contributor Author

As for how I ended up here, due to some arguments and a slight insomnia I ended up writing a Python module to enforce type hints on all imports at run-time and not as a CI step.

It turns out it ended up shooting itself as the importlib types didn't match reality <.<

But once that was fixed, it actually works, with only slight side-effects (may include nausea, sluggishness and ImportErrors.)

JelleZijlstra merged commit 1365926 into python:master Jan 30, 2022

JelleZijlstra commented Jan 30, 2022

Copy link
Copy Markdown
Member

Thank you! (And thanks Alex for improving the message)

Spindel deleted the stdlib-importlib-machinery-loader branch January 30, 2022 16:47
Cheaterman mentioned this pull request Feb 12, 2022
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

AlexWaygood AlexWaygood left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL