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

Support #noqa annotation #144

Closed
thpica opened this issue Jun 6, 2019 · 6 comments
Closed

Support #noqa annotation #144

thpica opened this issue Jun 6, 2019 · 6 comments
Labels
as designed Not a bug, working as intended enhancement request New feature or request

Comments

@thpica
Copy link

thpica commented Jun 6, 2019

Is your feature request related to a problem? Please describe.
There is no annotation to ignore linting errors on a line.

Describe the solution you'd like
Use the #noqa annotation to ignore linting errors on a line.

@thpica thpica added the enhancement request New feature or request label Jun 6, 2019
@erictraut
Copy link
Collaborator

Pyright doesn't perform any linting (code style) checks, only type checks.

There are already ways to provide additional type hints if needed. These include type annotations (including the "Any" type) and the cast function. So #noqa should not be necessary.

If you have an example of an error that you are not sure how to resolve, let me know and I'll try to offer some guidance.

@travisclagrone
Copy link

@erictraut Here's an example:

The #noqa annotation would be immensely useful in suppressing the pyright (reportMissingTypeStubs) message, such as for libraries that don't provide type stubs. E.g.

from sklearn.base import BaseEstimator, ClassifierMixin

It is entirely appropriate to both request that PyRight report missing type stubs, and explicitly disable said check in those cases where type stubs are not available. The current alternatives are both distasteful:

  1. Globally distable pyright (reportMissingTypeStubs), and thus hide potential "gotchas" (e.g. for libraries that do provide type stubs, but that require a separate package to be installed with them for some sort of performance purposes).
  2. Manually ignore warnings/errors due to pyright (reportMissingTypeStubs), and thus perpetuate a dangerous human-level tolerance of linter warnings/errors.

@erictraut
Copy link
Collaborator

As mentioned above, pyright doesn't perform any lifting checks, only type checks. The " # noqa" comment is really meant for linters. Pyright does support "# type: none" comments to suppress errors or warnings on a particular line. You can also use "# pyright: reportMissingTypeStubs: false" to disable this diagnostic for a single file. Does that address your issue?

@travisclagrone
Copy link

@erictraut The # pyright: reportMissingTypeStubs: false notation is sufficient; whether it's implemented with # noqa or some other directive isn't as important as the ability to do it somehow. I don't recall seeing the ability to do your suggestion mentioned in the user documentation, though... Could you please point me to it?

Also, the example I provided (i.e. pyright (reportMissingTypeStubs)) intentionally discusses the need for disabling a particular message on a particular line rather than a global-like disable for an entire file, which could easily (and silently!) clobber otherwise valid "other" (i.e. originally unknown) instances of the message (e.g. Scikit-Learn does not have type stubs whatsoever, while Django does but requires explicit installation of a separate package to get them). Similarly, the # type: ignore mechanism--which does at least target an individual line--clobbers all type checking for that line, which also bears the danger of silently clobbering otherwise valid messages.

Basically, what would address my issue (as well as what I think @thpica was originally getting at), is a way to disable a particular message on a particular line.

@erictraut
Copy link
Collaborator

Here's the documentation for comment-based directives supported by pyright:
https://github.com/microsoft/pyright/blob/master/docs/comments.md

@K3UL
Copy link

K3UL commented Sep 15, 2020

Hi @erictraut ! I would like to add something to the debate and suggest maybe reconsidering adding noqa support. Although noqa was added specifically for flake8/pep8, one could argue (I would) that type checking is also part of quality assurance.

I think it would make sense, when using a raw # noqa without any error number after, to disable all warnings : linting and type checks.
Also it would provide a more concise way to silent some false-positive. Currently the most straightforward is <code> # type: ignore which can often make the line go over the length limit. It can seem trivial but it DOES make life easier.

I know you are strongly against type: ignore so my guess is that you feel the same with noqa but the fact is that false positives DO exist (like json and typing, two things that don't do well together) and we are a lot to use this.

The only drawback I can think of is people who currently have noqa would have their type checking turning off all of a sudden, so this would have to be an opt-in behavior anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
as designed Not a bug, working as intended enhancement request New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants