Skip to content

Adds ability to have subclasses for NodeVisitor and TraverserVisitor #10125

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

Merged
merged 2 commits into from
Feb 26, 2021
Merged

Adds ability to have subclasses for NodeVisitor and TraverserVisitor #10125

merged 2 commits into from
Feb 26, 2021

Conversation

sobolevn
Copy link
Member

@sobolevn sobolevn commented Feb 21, 2021

Description

Currently a very important tool - which NodeVisitor definitely is - is not available to be used in a 3rd party code.
Because currently inheriting from NodeVisitor raises an exception: TypeError: interpreted classes cannot inherit from compiled traits

A developer has a single choice to replicate the same visitor patter by theyself, which is not really convenient.
So, I propose to make this consistent with TypeVisitor, which already allows having interpreted subclasses.

Refs a9fa9ab
Refs #9001
Refs #9602

Test Plan

As #9602 have shown, this is fine without tests.
Correct me if I am wrong, please, and I would contribute the required tests. CC @msullivan on this.

Currently a very important tool - which `NodeVisitor` definitely is - is not available to be used in a 3rd party code.
Because currently inheriting from `NodeVisitor` raises an exception: `TypeError: interpreted classes cannot inherit from compiled traits`

A developer has a single choice to replicate the same visitor patter by theyself, which is not really convenient.
So, I propose to make this consistent with `TypeVisitor`, which already allows having interpreted subclasses.

Refs a9fa9ab
Refs #9001
Refs #9602
Copy link
Collaborator

@TH3CHARLie TH3CHARLie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this as long as it wouldn't cause serious performance loss due to larger generated code.

@TH3CHARLie TH3CHARLie changed the title Adds ability to have subclasses for NodeVisitor Adds ability to have subclasses for NodeVisitor and TraverserVisitor Feb 21, 2021
@TH3CHARLie TH3CHARLie merged commit effd970 into python:master Feb 26, 2021
@zerlok
Copy link

zerlok commented May 20, 2021

When a version with these fixes will be released ?

@stuhood
Copy link

stuhood commented Aug 1, 2022

I don't think that this is working in Mypy 0.971:

from mypy.visitor import ExpressionVisitor

class SafePositionVisitor(ExpressionVisitor[None]):
    ...

...fails to import as a plugin with:

pyproject.toml:1:1: error: Error importing plugin "example":
interpreted classes cannot inherit from compiled traits  [misc]

@stuhood
Copy link

stuhood commented Apr 24, 2023

It looks like #10125 (comment) is due mypyc/mypyc#754. Thanks @JukkaL!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants