pre-commit / pygrep-hooks Goto Github PK

Hi,

In pandas, there's a local hook which checks for instances of, e.g.

    .. warning:

instead of

   .. warning::

:

    -   id: incorrect-sphinx-directives
        name: Check for incorrect Sphinx directives
        language: pygrep
        entry: |
            (?x)
            # Check for cases of e.g. .. warning: instead of .. warning::
            \.\.\ (
                autosummary|contents|currentmodule|deprecated|
                function|image|important|include|ipython|literalinclude|
                math|module|note|raw|seealso|toctree|versionadded|
                versionchanged|warning
            ):[^:]
        files: \.(py|pyx|rst)$

It's not specific to pandas, and others might find it useful - would a PR to include it in pygrep hooks be welcome?

The python-check-blanket-type-ignore hook catches # type: ignore [assignment] as a blanket type ignore, but mypy treats this as being specific:

$ cat t.py
a: int = 'string'  # type: ignore [misc]

$ mypy t.py
t.py:1: error: Incompatible types in assignment (expression has type "str", variable has type "int")  [assignment]
t.py:1: note: Error code "assignment" not covered by "type: ignore" comment
Found 1 error in 1 file (checked 1 source file)

The following comment produces a false match and therefore fails the python-use-type-annotations hook unexpectedly:

>>> import re
>>> re.search('# type(?!: *ignore([^a-zA-Z0-9]|$))', '# types are fun')
<re.Match object; span=(0, 6), match='# type'>

According to section Type comments of PEP 484 it looks to me like a type comment is always of the form # type: ... so I think the regex here

should actually be

entry: '# type:(?! *ignore([^a-zA-Z0-9]|$))'

with the : outside of the negative lookahead assertion. Happy to provide a PR, if necessary.

xref #33 (comment)

We have a method in our TM codebase that has the following comment, and I believe it is fine, but it is flagged by python-use-type-annotations (using the latest v1.10.0)

  def _filter_kwargs(self, **kwargs: Any) -> Dict[str, Any]:
      """Filter kwargs such that they match the update signature of the metric."""
      # filter all parameters based on update signature except those of
      # type VAR_POSITIONAL (*args) and VAR_KEYWORD (**kwargs)

For example:

# type is a keyword in python, so don't name a variable that
ticket_type = models.NullCharField(
    max_length=255,
    db_column='type',
)

I think we should have the regex check for comments starting with type: instead of just type.

I have a function with a docstring that looks like this:

def foo(m: Mock, *args, **kwargs):
    """
    ...

    This function is similar to :meth:`Mock.assert_called_once_with`,
    but ...

    ....
    """

python-check-mock-methods complains about this:

tests/conftest.py:86:    This function is similar to :meth:`Mock.assert_called_once_with`,

I don't understand that, because that method exists.

Hi! The project doesn't include a license file, is that intentional? Would I be allowed to copy the same regex patterns to a different project?

mypy allows ignoring an entire file by adding a # type: ignore comment at the top of the file: python/mypy#964.

Thoughts on updating it to catch other common typos like:

<func>.called_once_with() without an assert in front?

I'm happy to make a PR for this!

Good day!
Thank you for creating this tool!

Can you please support # NOQA as well? It's respected by flake8 linter by goes undetected by this hook.

Thank you,
Zaar

Hope you can help, I have configured pre-commit and this is my .pre-commit-config.yaml

repos:
  - repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v4.3.0
    hooks:
      - id: check-added-large-files
        args: [--maxkb=2500]
      - id: check-ast
      - id: check-case-conflict
      - id: check-docstring-first
      - id: check-json
      - id: check-merge-conflict
      - id: check-toml
      - id: check-yaml
      - id: end-of-file-fixer
      - id: name-tests-test
        args: [--pytest-test-first]
      - id: no-commit-to-branch
        args: [--branch, develop, --branch, master, --branch, adf_release]
  - repo: https://github.com/pre-commit/mirrors-yapf
    rev: v0.32.0
    hooks:
      - id: yapf
        additional_dependencies: [toml]
        args: [--style "google"]
  - repo: https://github.com/psf/black
    rev: 22.6.0
    hooks:
      - id: black
        args: [--line-length, "180"]
  - repo: https://gitlab.com/pycqa/flake8
    rev: 4.0.1
    hooks:
      - id: flake8
        args: [--docstring-convention, google]
        additional_dependencies: [flake8-bugbear, flake8-docstrings]
  - repo: https://github.com/pycqa/pylint
    rev: v2.14.5
    hooks:
      - id: pylint
  - repo: https://github.com/pre-commit/pygrep-hooks
    rev: v1.9.0  # Use the ref you want to point at
    hooks:
      - id: python-use-type-annotations
  - repo: https://github.com/pycqa/bandit
    rev: 1.7.4
    hooks:
      - id: bandit
        files: src/

All my other hooks seem to be fine, but I have funtions in my code at the moment without types, e.g.

def get_base_name(file):
    """Returns file basename from file path.

    Args:
        file (): File path

    Returns:
        [str]:File basename
    """
    return os.path.basename(file)

And these do not seem to be getting picked up by the hook? When I run pre-commit with --verbose I get (commented out the other hooks for this output)

pre-commit run --all-files --verbose
type annotations not comments............................................Passed
- hook id: python-use-type-annotations
- duration: 0.2s

So the hook seems to run, but doesn't appear to have checked/run on anythiing.

Hope you have an idea.

Thanks

Karl

Any solutions?

Originally posted by @tuhin1729 in pre-commit/demo-repo#7 (comment)

For example, model.eval() gets matched.

The regex should be changed to something like (?<!(\w\.))eval\(

The spelling here is missing the closing parenthesis.

It looks a bit jarring when used together with check for eval().

check for eval().........................................................Passed
use logger.warning(......................................................Passed

I'm not if it might be intentional (I don't see an indication of it), but the rst-directive-colons hook has types: [text] instead of rst like the other two reST hooks. Therefore, if unintentional, false positives might result, and it also doesn't show up when rst is selected on the pre-commit hooks page (by the way, was that dropdown a relatively new addition or did I just miss it before? Its really nice to have, thanks, and its a further motivation for me to help upgrade older hooks to use types instead of files wherever practicable).

Happy to submit a PR to fix this if it isn't intended, but I wanted to ask first. Thanks!

I have a snippet of code causing problems with python-no-log-warn, particularly the distutils log object has only a warn method, and not warning. What is the recommended way of ignoring the check for this specific instance?

The standard library package warning has a .warn() method. This check incorrectly blocks when that method is used.

The description states this is a "A quick check for the deprecated .warn() method of python loggers."

As a user, I would expect this check to only check logger objects for usages of the .warn method. What I experience is that warnings.warn() is being flagged.

Hi,

since the detection for the groups:

- id: rst-backticks
- id: rst-directive-colons
- id: rst-inline-touching-normal

is really precise, is it possible to add an option to auto-fix the found locations? :)

See pypa/pip#10914, especially pypa/pip#10914 (comment).

We have Markdown stuff inside an RST code block, and there're Markdown backticks in there. However, the rst-backticks hook thinks those backticks are part of the doc (not the code block), so it fails. Can this check be smarter to avoid checking code blocks?

The python-no-eval hook generates false positives for PyTorch models' eval method [1].

[1] https://pytorch.org/docs/stable/generated/torch.nn.Module.html#torch.nn.Module.eval

Since a recent version mypy allows you to silence only specific error codes with #type: ignore[attr-undefined]. These get flagged as type comments.. See the first note in https://mypy.readthedocs.io/en/stable/common_issues.html#spurious-errors-and-locally-silencing-the-checker

First thanks a lot for this hooks !
I use them and find them really useful.

The only one not matching my usecase is the "eval check one".

I have classes with my_instance.eval(my_params) and it raise errors for them.
Instead I guess it should only check for eval(params) built-in function ?

thanks in advance,
have a great day.