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

Now OpExpr.op is a enum #11263

Closed
wants to merge 6 commits into from
Closed

Now OpExpr.op is a enum #11263

wants to merge 6 commits into from

Conversation

sobolevn
Copy link
Member

@sobolevn sobolevn commented Oct 4, 2021

I hope this this would help to maintain simplier and more safe codebase.

@97littleleaf11
Copy link
Collaborator

How's the performance of Enum in current CPython? I just notice that python/cpython#17669 hasn't been merged yet.

@sobolevn
Copy link
Member Author

sobolevn commented Oct 4, 2021

@97littleleaf11 do you have any specific benchmark in mind?

@sobolevn
Copy link
Member Author

sobolevn commented Oct 4, 2021

@97littleleaf11 one more question (looks like you are quite familiar with mypyc, unlike me): do you know why this failure happens?

mypy/test/testapi.py:6: in <module>
    from mypy.test.helpers import Suite
mypy/test/helpers.py:19: in <module>
    from mypy.main import process_options
mypy/main.py:13: in <module>
    from mypy import build
mypy/build.py:29: in <module>
    from mypy.nodes import MypyFile, ImportBase, Import, ImportFrom, ImportAll, SymbolTable
mypy/nodes.py:17: in <module>
    from mypy.operators import BinOp
mypy/operators.py:8: in <module>
    class BinOp(str, enum.Enum):
E   KeyError: 'str'

https://github.com/python/mypy/pull/11263/checks?check_run_id=3790982937

It looks like some mypyc bug / limitation, but I have no idea what is going on.

@JukkaL
Copy link
Collaborator

JukkaL commented Oct 4, 2021

To be honest, I don't think that is worth it, since mypyc's enum support is still incomplete.

Also, I kind of like being able to refer to operations using literals, since there's no ambiguity. This requires remembering (or looking up) the operator vs enum item name mapping.

@sobolevn
Copy link
Member Author

sobolevn commented Oct 4, 2021

To be honest, I don't think that is worth it, since mypyc's enum support is still incomplete.

Ok, fair enough. I see that this is the second PR I am struggling with Enum and mypyc compat. I will just remove TODO items in some next PR.

@sobolevn sobolevn closed this Oct 4, 2021
@97littleleaf11
Copy link
Collaborator

@97littleleaf11 one more question (looks like you are quite familiar with mypyc, unlike me): do you know why this failure happens?

mypy/test/testapi.py:6: in <module>
    from mypy.test.helpers import Suite
mypy/test/helpers.py:19: in <module>
    from mypy.main import process_options
mypy/main.py:13: in <module>
    from mypy import build
mypy/build.py:29: in <module>
    from mypy.nodes import MypyFile, ImportBase, Import, ImportFrom, ImportAll, SymbolTable
mypy/nodes.py:17: in <module>
    from mypy.operators import BinOp
mypy/operators.py:8: in <module>
    class BinOp(str, enum.Enum):
E   KeyError: 'str'

https://github.com/python/mypy/pull/11263/checks?check_run_id=3790982937

It looks like some mypyc bug / limitation, but I have no idea what is going on.

I think we don't support inheriting from str currently

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.

3 participants