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

AssertionError: Internal error: too many class plugin hook passes #15004

Closed
mementum opened this issue Apr 3, 2023 · 5 comments · Fixed by #15157
Closed

AssertionError: Internal error: too many class plugin hook passes #15004

mementum opened this issue Apr 3, 2023 · 5 comments · Fixed by #15157

Comments

@mementum
Copy link

mementum commented Apr 3, 2023

Crash Report

See the traceback.

Traceback

Traceback (most recent call last):
  File "D:\dro\bin\WPy64-31090\python-3.10.9.amd64\lib\runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "D:\dro\bin\WPy64-31090\python-3.10.9.amd64\lib\runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "D:\dro\bin\WPy64-31090\python-3.10.9.amd64\scripts\mypy.exe\__main__.py", line 7, in <module>
  File "D:\dro\bin\WPy64-31090\python-3.10.9.amd64\lib\site-packages\mypy\__main__.py", line 15, in console_entry
    main()
  File "mypy\main.py", line 95, in main
  File "mypy\main.py", line 174, in run_build
  File "mypy\build.py", line 194, in build
  File "mypy\build.py", line 277, in _build
  File "mypy\build.py", line 2923, in dispatch
  File "mypy\build.py", line 3320, in process_graph
  File "mypy\build.py", line 3415, in process_stale_scc
  File "mypy\semanal_main.py", line 99, in semantic_analysis_for_scc
  File "mypy\semanal_main.py", line 429, in apply_class_plugin_hooks
AssertionError: Internal error: too many class plugin hook passes

To Reproduce

#!/usr/bin/env python
# -*- coding: utf-8; py-indent-offset:4 -*-
###############################################################################
from __future__ import annotations
from dataclasses import asdict, dataclass, field
from decimal import Context, Decimal, Inexact
from typing import ClassVar, Protocol


DecimalPlus = Decimal | str | int | float


@dataclass
class TargetSettings(Protocol):
    max: DecimalPlus
    min: DecimalPlus
    inc: DecimalPlus
    precision: int

    PLACES: Decimal = field(init=False, repr=False)
    INEXACT: ClassVar[Context] = Context(traps=[Inexact])

    def __post_init__(self) -> None:
        self.PLACES = PLACES = Decimal(10) ** -self.precision

        self.min = Decimal(self.min).quantize(PLACES)
        self.max = Decimal(self.max).quantize(PLACES)
        self.inc = Decimal(self.inc).quantize(PLACES)

    def is_valid(self, val: DecimalPlus) -> bool:
        try:
            val = Decimal(val).quantize(self.PLACES, context=self.INEXACT)
        except Inexact:
            return False  # more decimal places than allowed

        # mypy does not know attributes are Decimals at this stage
        if not (self.min <= val <= self.max):  # type: ignore
            return False

        return True  # all checks passed

Your Environment

  • Mypy version used: 1.1.1
  • Mypy command-line flags: None
  • Mypy configuration options from mypy.ini (and other config files): None
  • Python version used: 3.10.9
  • Operating system and version: Windows 11 22621.1413

Additional Comment

Let me say that I do believe that the concept of static type checking of Union is broken (again: the concept). In my very humble opinion as user, if I do declare something as a Union (either with the keyword or the | operator), mypy must believe that when the time comes, the variable/attribute declared as a Union, will contain one of the types.

Reporting errors like

error: Unsupported operand types for <= ("Decimal" and "str")  [operator]

when the Union contains Union[Decimal, str, int, float] is in my very humble opinion: wrong. Such an error can be reported during runtime, but not during static analysis, because mypy has no way of knowing what the type at that stage contains. Reporting a "warning" would be ok. Reporting an "error" is an error itself.

No, as a user I am not in the mood of adding cast here and there and here-there to pollute the code, hence the need to add a # type: ignore to silence mypy.

@mementum mementum added the crash label Apr 3, 2023
@JelleZijlstra
Copy link
Member

Here's a smaller reproducer:

from dataclasses import dataclass
from typing import Protocol

@dataclass
class TargetSettings(Protocol):
    min: int

    def __post_init__(self) -> None:
        self.min = int(self.min)

Combining @dataclass and Protocol is a strange thing to do and I'm not sure it should be allowed. However, mypy should not crash.

@mementum
Copy link
Author

mementum commented Apr 4, 2023

I do understand that @dataclass may make no sense for a Protocol, but this is, imho, bound to happen again, assuming typing becomes the mainstream development path.

How it happened in this case:

  • A dataclass ended up being used by a secondary class class and mypy was complaining that the referenced dataclass didn't have attribute X. Because it was defined later in the code.
  • The dataclass vital parts were taken out and put into a Protocol defined before the secondary class which means the secondary class can reference the protocol and be happy
  • Because the final class is a dataclass it seemed sensible to also have the Protocol class being decorated as a dataclass

@WISEPLAT

This comment was marked as spam.

@mementum
Copy link
Author

mementum commented Apr 8, 2023

One of the reasons to dive deep into typing is to modernize backtrader with typing and also add additional features from more modern Python.

Python 2.7 was still trendy and full in scope back in 2015.

@WISEPLAT

This comment was marked as spam.

JukkaL pushed a commit that referenced this issue May 2, 2023
Fix #15004 

FWIW I don't think dataclass protocols make much sense, but we
definitely should not crash. Also the root cause has nothing to do with
dataclasses, the problem is that a self attribute assignment in a
protocol created a new `Var` (after an original `Var` was created in
class body), which is obviously wrong.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants