mypy icon indicating copy to clipboard operation
mypy copied to clipboard

Refurb crashes on Mypy 1.7.0

Open dosisod opened this issue 2 years ago • 3 comments

Bug Report

Refurb is a project that depends on Mypy internals to work properly. Since v1.7.0 of Mypy (specifically since https://github.com/python/mypy/pull/15770 was merged), Refurb no longer works, and instead emits the following error:

$ refurb file.py
interpreted classes cannot inherit from compiled traits

See also: https://github.com/dosisod/refurb/issues/305

To Reproduce

$ pip install refurb==1.22.2 mypy==1.7.0
$ touch file.py
$ refurb file.py
interpreted classes cannot inherit from compiled traits

Expected Behavior

Refurb doesn't crash.

Actual Behavior

Refurb is crashing.

Your Environment

  • Mypy version used: 1.7.0
  • Python version used: 3.11.5

Background

#15770 added a @trait decorator to TraverserVisitor, meaning 3rd party (interpreted) programs can no longer inherit from TraverserVisitor:

https://github.com/python/mypy/blob/c6cb3c6282003dd3dadcf028735f9ba6190a0c84/mypy/traverser.py#L97-L99

I tried a bunch of different workarounds including making a custom __new__ method, copy-and-pasting TraverserVisitor into my code and removing the @trait, but alas nothing is working, and so I had to pin Mypy to <= v1.6.1 in Refurb, which will prevent users from using the newest version of Mypy with Refurb. Using the non-compiled version of Mypy doesn't have this issue, but doing so would be far too slow, especially since Refurb parses/walks the full, fine-grained AST tree (similar to mypyd).

My question: How should I get around this? Is there anything I can do on my end, or does something in Mypy have to change? I would think that allow_interpreted_subclasses=True would nullify the inheritance restriction imposed by @trait, but that doesn't seem to be the case.

There is also the bigger question of how 3rd parties should safely use Mypy internals (or if they even should in the first place). Currently Mypy internals are not versioned and can change with any release. In addition, certain parts of Mypy are hard/impossible to use outside of Mypy itself, whether that's because they don't work, crash, or require lots of moving parts because they weren't meant to be used in a stand-alone environment. I know that standardizing/stabilizing Mypy's internals so that 3rd parties can use them is probably not a major priority, but I thought I would bring it up to gauge how you all feel about it. I could elaborate more but I want to keep this short. I can open a separate issue for this if need be.

dosisod avatar Nov 15 '23 08:11 dosisod

This issue is also of interest to Debian, so a hack to fix this (here and/or in refurb) would be appreciated!

mr-c avatar Nov 16 '23 15:11 mr-c

I would think that allow_interpreted_subclasses=True would nullify the inheritance restriction imposed by @trait, but that doesn't seem to be the case.

I definitely remember we wanted to allow this, but probably this is still not done. Anyway, an idea worth trying is to move @trait from TraverserVisitor to BaseStubGenerator. I guess there are more people who want to inherit from the former, while the latter is quite niche.

ilevkivskyi avatar Nov 17 '23 10:11 ilevkivskyi

I'm running into this issue as well - developed a plugin that makes use of TraverserVisitor and didn't realize that it would crash using the compiled version. For now I'm considering the somewhat ugly workaround of simply copying the TraverserVisitor source so it's able to be interpreted...

mike-kaoudis-asimov avatar Jun 21 '24 02:06 mike-kaoudis-asimov

same here, for https://github.com/pyastrx/pyastrx Have you found any workarounds? @mike-kaoudis-asimov @dosisod @mr-c

devmessias avatar Sep 30 '24 13:09 devmessias

Have you found any workarounds?

Over at Debian? No, we didn't find any workaround and the refurb package got removed from Debian Testing due to that

mr-c avatar Oct 01 '24 11:10 mr-c

Ivan made a suggestion above (and Mike also suggested simply vendoring the visitor). If someone makes a PR, tag me and I'll review it.

hauntsaninja avatar Oct 01 '24 18:10 hauntsaninja

Sorry for your loss @mr-c :cry: I would recommend Pyre; it is very fast, but I was disappointed because they have now deprecated the only feature that was useful for me. Pyre Query Documentation.

devmessias avatar Oct 01 '24 18:10 devmessias

@hauntsaninja I don't get. The vendoring solution for this issue must be done in the refurb pacakge right?

devmessias avatar Oct 01 '24 18:10 devmessias

Ivan's suggestion above could be done in mypy.

JelleZijlstra avatar Oct 01 '24 18:10 JelleZijlstra

I ended up writing a new class with a very similar implementation to TraverserVisitor, but was forced to invert the visitor pattern as Node.accept(<my_new_class>) wouldn't work no matter what. Here's a paste with what I did - would be happy to open a pr to contribute it back if that's desired, but it'd probably be preferable to figure out the allow_interpreted_subclasses issue if possible (beyond my abilities). If you do use this, definitely be warned that changes to the functionality or shape of any Node classes will break it in unpredictable ways.

mike-kaoudis-asimov avatar Oct 01 '24 18:10 mike-kaoudis-asimov

@JelleZijlstra I just made a search here, why this will not create the same issue in InspectionStubGenerator? image

devmessias avatar Oct 01 '24 19:10 devmessias

Hmm maybe we could compile stubgen modules, so they wouldn't need to use interpreted subclasses?

JukkaL avatar Oct 01 '24 19:10 JukkaL

Or alternatively, would it help if we'd stop compiling mypy.stubutil.

JukkaL avatar Oct 01 '24 19:10 JukkaL

Well, wouldn't it also be possible to have an ugly solution with duplicated code TraverserVisitor (with trait) and TraverserVisitorExt

devmessias avatar Oct 01 '24 19:10 devmessias

@devmessias Yeah, that would work, but some of the other ideas wouldn't require any code duplication.

JukkaL avatar Oct 01 '24 19:10 JukkaL

FWIW in my answer to this StackOverflow question of Is it possible to get the inferred type information using mypy programmatically?, I've worked around the issue of subclassing mypy classes using a custom meta path finder to skip mypy's C extension and load mypy's Python modules directly:

import os
import sys
from importlib.util import spec_from_loader
from importlib.machinery import PathFinder, SourceFileLoader

class MypySourceFinder:
    mypy_module_path, = PathFinder().find_spec('mypy').submodule_search_locations

    def find_spec(self, fullname, path=None, target=None):
        package, *names = fullname.split('.')
        if package == 'mypy':
            path = self.mypy_module_path
            for name in names:
                if not os.path.isdir(subpackage_path := os.path.join(path, name)):
                    break
                path = subpackage_path
            else:
                name = '__init__'
            return spec_from_loader(
                fullname,
                SourceFileLoader(fullname, os.path.join(path, name + '.py'))
            )

sys.meta_path.insert(0, MypySourceFinder())

from mypy.nodes import Node
print(type(Node.__init__)) # outputs <class 'function'> rather than <class 'wrapper_descriptor'>

class MyNode(Node): ...
MyNode() # OK, not raising TypeError: interpreted classes cannot inherit from compiled

blhsing avatar Nov 12 '24 05:11 blhsing

The workaround aside, what is the reason, technical or conceptual, behind why these mypy classes have to explicitly disallow inheritance by interpreted subclasses in the first place? It otherwise seems like an unnecessary and artificial limitation to me.

blhsing avatar Nov 13 '24 01:11 blhsing