dosisod / refurb

A tool for refurbishing and modernizing Python codebases
GNU General Public License v3.0
2.48k stars 54 forks source link

[Bug]: crash within `mypy` on `sympy/polys/numberfields/resolvent_lookup.py` #302

Closed jamesbraza closed 11 months ago

jamesbraza commented 11 months ago

Has your issue already been fixed?

The Bug

I have sympy==1.12 installed as it's a dependency of torch==2.1.0. Running refurb on some files:

> refurb dir1 file.py
/usr/local/lib/python3.11/site-packages/sympy/polys/numberfields/resolvent_lookup.py: error: INTERNAL ERROR -- Please try using mypy master on GitHub:
https://mypy.readthedocs.io/en/stable/common_issues.html#using-a-development-mypy-build
If this issue continues with mypy master, please report a bug at https://github.com/python/mypy/issues
version: 1.6.1
/usr/local/lib/python3.11/site-packages/sympy/polys/numberfields/resolvent_lookup.py: : note: please use --show-traceback to print a traceback when reporting a bug

Running mypy on the same files does not have this error. I want to use --show-traceback so I can properly debug this issue (perhaps report the crash to mypy).

Here is some more info on the Docker container:

> uname -a
Linux 26cbe3fc0bf0 5.15.49-linuxkit #1 SMP PREEMPT Tue Sep 13 07:51:32 UTC 2022 aarch64 GNU/Linux
> dpkg --print-architecture
arm64

Version Info

Refurb: v1.22.1
Mypy: v1.6.1

Python Version

3.11.5

Config File

[tool.mypy]
check_untyped_defs = true
error_summary = false
platform = "linux"
pretty = true
show_column_numbers = true
show_error_codes = true
show_error_context = true
warn_redundant_casts = true
warn_unreachable = true
warn_unused_configs = true
warn_unused_ignores = true

[[tool.mypy.overrides]]
ignore_missing_imports = true
module = [
    "fastapi_socketio",  # SEE: https://github.com/pyropy/fastapi-socketio/issues/27
    "fsspec",  # SEE: https://github.com/fsspec/filesystem_spec/issues/625
    "huggingface_hub",  # SEE: https://github.com/huggingface/huggingface_hub/issues/1662
    "rapidjson",  # SEE: https://github.com/python-rapidjson/python-rapidjson/issues/190
    "s3fs",  # SEE: https://github.com/fsspec/s3fs/issues/383
    "setuptools_scm",  # SEE: https://github.com/pypa/setuptools_scm/issues/501
    "socketio.*",  # SEE: https://github.com/miguelgrinberg/python-socketio/issues/1276
    "streamlit_pydantic",
    "transformers",
]

[tool.refurb]
enable_all = true
ignore = [
    "FURB103",
    "FURB141",
    "FURB144",
    "FURB147",
    "FURB150",
    "FURB155",
]

Extra Info

Here is the relevant sympy file: resolvent_lookup.py

Running mypy on the file directly:

> touch a.py
> # Copy resolvent_lookup.py into a.py
> mypy a.py

It doesn't seem to crash or have an error, I am not sure. Trying the current head of sympy:

> pip install git+https://github.com/sympy/sympy.git
...
Successfully installed sympy-1.13.dev0
> refurb dir1 file.py
# Same error as above
jamesbraza commented 11 months ago

How can I pass --show-traceback to mypy so we can learn more about this crash? Any chance refurb exposes some flag for this?

dosisod commented 11 months ago

Thank you for opening this! This is a weird crash, I will see if I can reproduce this locally.

The README actually mentions this use-case exactly:

Overriding Mypy Flags

This is typically used for development purposes, but can also be used to better fine-tune Mypy from within Refurb. Any command line arguments after -- are passed to Mypy. For example:

$ refurb files -- --show-traceback

I'd be curious to see what that traceback looks like.

dosisod commented 11 months ago

Well, I think I found the problem:

...

  File "mypy/traverser.py", line 263, in visit_op_expr
  File "mypy/nodes.py", line 2050, in accept
  File "/home/loot/git/refurb/refurb/visitor/visitor.py", line 21, in inner
    getattr(TraverserVisitor, name)(self, o)
  File "mypy/traverser.py", line 263, in visit_op_expr
  File "mypy/nodes.py", line 2050, in accept
  File "/home/loot/git/refurb/refurb/visitor/visitor.py", line 19, in inner
    self.run_check(o, check)
  File "/home/loot/git/refurb/refurb/visitor/visitor.py", line 64, in run_check
    check(node, self.errors, self.settings)  # type: ignore
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/loot/git/refurb/refurb/checks/builtin/use_isinstance_tuple.py", line 46, in check
    match extract_binary_oper("or", node):
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
RecursionError: maximum recursion depth exceeded

I opened the resolvent_lookup.py file, and even my IDE gives up trying to parse it:

image

There are 2 ways of getting around this: Increase the recursion limit, or skip the file entirely. I think skipping the file is the best solution because there probably isn't much use in trying to parse/cleanup code like this. I'll go ahead and fix this later today.

jamesbraza commented 11 months ago

Is skipping the file something I should do in a config file, or are you going to do something within refurb itself?

dosisod commented 11 months ago

I should've been more specific, Refurb should automatically skip a file if RecursionError is raised. As a workaround you can tell Mypy to exclude any problematic files:

$ refurb some files here -- --exclude problematic/file/here.py
jamesbraza commented 11 months ago

Sounds good! Sorry my earlier reply was a bit brief, I actually hit send before I was finished drafting. Should have begun with thanks for the speedy response and helping root cause the issue!

Good to know there's a way to pass through to mypy flags, thanks. It does indeed work in showing the RecursionError:

> refurb dir1 file.py -- --show-traceback
...
  File "/usr/local/lib/python3.11/site-packages/mypy/fastparse.py", line 402, in visit
    return visitor(node)
           ^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/mypy/fastparse.py", line 1406, in visit_BinOp
    e = OpExpr(op, self.visit(n.left), self.visit(n.right))
                   ^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/mypy/fastparse.py", line 402, in visit
    return visitor(node)
           ^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/mypy/fastparse.py", line 1406, in visit_BinOp
    e = OpExpr(op, self.visit(n.left), self.visit(n.right))
                   ^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/mypy/fastparse.py", line 402, in visit
    return visitor(node)
           ^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/mypy/fastparse.py", line 1401, in visit_BinOp
    op = self.from_operator(n.op)
         ^^^^^^^^^^^^^^^^^^^^^^^^
RecursionError: maximum recursion depth exceeded
/usr/local/lib/python3.11/site-packages/sympy/polys/numberfields/resolvent_lookup.py: : note: use --pdb to drop into pdb

I am not quite able to get exclude to work, none of these are working for me

refurb dir1 file.py -- --exclude=resolvent_lookup\.py$
refurb dir1 file.py -- --exclude=numberfields

I will just wait until the RecursionError bug fix

dosisod commented 11 months ago

@jamesbraza Just fixed this! The new version should be available on PyPi in a couple of minutes.

jamesbraza commented 11 months ago

It seems to have worked in resolving that issue, so thank you!! And I appreciate the bugfix release for this 👍

Unfortunately, for me it seems another issue is now exposed, see https://github.com/dosisod/refurb/issues/305