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

Adding an --ignore-missing-type-hints config #10660

Closed
evinism opened this issue Jun 16, 2021 · 5 comments
Closed

Adding an --ignore-missing-type-hints config #10660

evinism opened this issue Jun 16, 2021 · 5 comments
Labels

Comments

@evinism
Copy link

evinism commented Jun 16, 2021

Feature

We introduce a new configuration option, namely --ignore-missing-type-hints as a lighter alternative to --ignore-missing-imports. Such a config would be allowed in both the global level and to package-specific configs.

The config would simply disable "Skipping analyzing "offending.package": found module but no type hints or library stubs" error messages. More specifically, we'd be disabling errors on ModuleNotFoundReason.FOUND_WITHOUT_TYPE_HINTS.

Pitch

Having to ignore-missing-imports each individual package I import that hasn't built type hints is really frustrating. Several of the notes here for resolving this error are largely non-actionable -- many libraries don't have types associated with them, nor stubs in typeshed.

There seem to be a few related issues, for example #4542, #9789 (thanks @hauntsaninja)

Given that a specific ModuleNotFoundReason exists, it makes me wonder why this flag hasn't been implemented yet -- is there a specific technical reason for it?

@evinism
Copy link
Author

evinism commented Jun 16, 2021

For reference, here's the incomplete idea of what i'm going for in my fork: evinism#1

If it turns out there's no reason this hasn't been implemented, I'll brush it up and submit a PR.

@JukkaL
Copy link
Collaborator

JukkaL commented Jun 17, 2021

My first impression is that the overall idea sounds reasonable. We may want to bikeshed some of the details though.

I haven't thought about this carefully yet, but here are some random thoughts:

  • We could plausibly introduce a special error code for these errors. It would then be possible to ignore the error code. However, this would be a backward incompatible change, unless we also add support for a hierarchy of error codes. If we'd do that, the new error could be a sub-error of the original error code, and ignoring the original error code would also ignore the sub-error.
  • This would apply to at least installed packages without type hints. We could also make this work with known third-party packages for which we ask the user to install a types-* package. However, the latter case is more actionable, so perhaps it shouldn't be covered.

@hauntsaninja
Copy link
Collaborator

Related: #9789

@evinism
Copy link
Author

evinism commented Jun 18, 2021

@JukkaL Good to know! Forgive me if these questions don't make any sense, i'm very new to this project.

First bullet

For (1), to make it more concrete, I take it that what you're proposing is either:

  • 1a. splitting the import error code into two, for example import and has-stubs backwards incompatibly, or
  • 1b. creating a new abstraction on top of error codes to support sub-errors, e.g. import which has import:has-stubs and import:no-missing-module as suberrors

Is this interpretation correct?

To nail it down further, as i understand, the user benefit of adding in an error code representation is that we can add inline annotations to ignore it, e.g. # type: ignore[import:has-stubs] in the code. Is that interpretation also correct?

I wonder if there's other uses for sub-errors that haven't been explored yet, or what other static analyzers do, etc. Has such a thing been brought up before?

Second Bullet

For (2), just from a 'least surprising behavior' perspective, i'd imagine that --ignore-missing-type-hints would also ignore missing type hints for which there is an easy way to get those type hints. I'd imagine that'd be a good thing to get right first time.

@hauntsaninja
Copy link
Collaborator

Thanks for the feature request. I'm going to close this as a duplicate of #9789. I opened a PR with a proposed implementation here: #14740

@hauntsaninja hauntsaninja closed this as not planned Won't fix, can't repro, duplicate, stale Feb 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants