hgrecco / pint

Operate and manipulate physical quantities in Python
http://pint.readthedocs.org/
Other
2.4k stars 469 forks source link

Python 3.13 error: cannot inherit frozen dataclass from a non-frozen one #1969

Open simonw opened 6 months ago

simonw commented 6 months ago

Running Pint against the current Python 3.13 developer preview throws this error:

pint/__init__.py:18: in <module>
    from .delegates.formatter._format_helpers import formatter
pint/delegates/__init__.py:12: in <module>
    from . import txt_defparser
pint/delegates/txt_defparser/__init__.py:12: in <module>
    from .defparser import DefParser
pint/delegates/txt_defparser/defparser.py:10: in <module>
    from . import block, common, context, defaults, group, plain, system
pint/delegates/txt_defparser/common.py:23: in <module>
    @dataclass(frozen=True)
../../../.pyenv/versions/3.13-dev/lib/python3.13/dataclasses.py:1247: in wrap
    return _process_class(cls, init, repr, eq, order, unsafe_hash,
../../../.pyenv/versions/3.13-dev/lib/python3.13/dataclasses.py:1015: in _process_class
    raise TypeError('cannot inherit frozen dataclass from a '
E   TypeError: cannot inherit frozen dataclass from a non-frozen one
simonw commented 6 months ago

Steps to reproduce:

cd /tmp
git clone https://github.com/hgrecco/pint
cd pint
python3.13 -m venv venv
# Or for me that was:
# ~/.pyenv/versions/3.13-dev/bin/python -m venv venv
source venv/bin/activate
python -m pip install pytest
python -m pip install -e .
pytest
simonw commented 6 months ago

It's possible this issue is related:

See cPython commit: https://github.com/python/cpython/commit/b6000d287407cbccfbb1157dc1fc6128497badc7

simonw commented 6 months ago

I think this is the code that the trackback complains about: https://github.com/hgrecco/pint/blob/f2e4081aee38f850938048beac7fb69c4908bc5e/pint/delegates/txt_defparser/common.py#L15-L25

And yes, DefinitionSyntaxError() there is marked as frozen=True - but the errors.DefinitionSyntaxError class it extends is marked frozen=False here:

https://github.com/hgrecco/pint/blob/f2e4081aee38f850938048beac7fb69c4908bc5e/pint/errors.py#L105-L115

simonw commented 6 months ago

Modifying pint/errors.py and replacing every instance of @dataclass(frozen=False) with @dataclass(frozen=True) addresses this problem, but I don't know if it's the right solution.

diff --git a/pint/errors.py b/pint/errors.py
index 59d3b45..f080f52 100644
--- a/pint/errors.py
+++ b/pint/errors.py
@@ -81,12 +81,12 @@ class WithDefErr:
         return DefinitionError(self.name, self.__class__, msg)

-@dataclass(frozen=False)
+@dataclass(frozen=True)
 class PintError(Exception):
     """Base exception for all Pint errors."""

-@dataclass(frozen=False)
+@dataclass(frozen=True)
 class DefinitionError(ValueError, PintError):
     """Raised when a definition is not properly constructed."""

@@ -102,7 +102,7 @@ class DefinitionError(ValueError, PintError):
         return self.__class__, tuple(getattr(self, f.name) for f in fields(self))

-@dataclass(frozen=False)
+@dataclass(frozen=True)
 class DefinitionSyntaxError(ValueError, PintError):
     """Raised when a textual definition has a syntax error."""

@@ -115,7 +115,7 @@ class DefinitionSyntaxError(ValueError, PintError):
         return self.__class__, tuple(getattr(self, f.name) for f in fields(self))

-@dataclass(frozen=False)
+@dataclass(frozen=True)
 class RedefinitionError(ValueError, PintError):
     """Raised when a unit or prefix is redefined."""

@@ -130,7 +130,7 @@ class RedefinitionError(ValueError, PintError):
         return self.__class__, tuple(getattr(self, f.name) for f in fields(self))

-@dataclass(frozen=False)
+@dataclass(frozen=True)
 class UndefinedUnitError(AttributeError, PintError):
     """Raised when the units are not defined in the unit registry."""

@@ -150,13 +150,13 @@ class UndefinedUnitError(AttributeError, PintError):
         return self.__class__, tuple(getattr(self, f.name) for f in fields(self))

-@dataclass(frozen=False)
+@dataclass(frozen=True)
 class PintTypeError(TypeError, PintError):
     def __reduce__(self):
         return self.__class__, tuple(getattr(self, f.name) for f in fields(self))

-@dataclass(frozen=False)
+@dataclass(frozen=True)
 class DimensionalityError(PintTypeError):
     """Raised when trying to convert between incompatible units."""

@@ -183,7 +183,7 @@ class DimensionalityError(PintTypeError):
         return self.__class__, tuple(getattr(self, f.name) for f in fields(self))

-@dataclass(frozen=False)
+@dataclass(frozen=True)
 class OffsetUnitCalculusError(PintTypeError):
     """Raised on ambiguous operations with offset units."""

@@ -208,7 +208,7 @@ class OffsetUnitCalculusError(PintTypeError):
         return self.__class__, tuple(getattr(self, f.name) for f in fields(self))

-@dataclass(frozen=False)
+@dataclass(frozen=True)
 class LogarithmicUnitCalculusError(PintTypeError):
     """Raised on inappropriate operations with logarithmic units."""

@@ -233,7 +233,7 @@ class LogarithmicUnitCalculusError(PintTypeError):
         return self.__class__, tuple(getattr(self, f.name) for f in fields(self))

-@dataclass(frozen=False)
+@dataclass(frozen=True)
 class UnitStrippedWarning(UserWarning, PintError):
     msg: str

@@ -241,13 +241,13 @@ class UnitStrippedWarning(UserWarning, PintError):
         return self.__class__, tuple(getattr(self, f.name) for f in fields(self))

-@dataclass(frozen=False)
+@dataclass(frozen=True)
 class UnexpectedScaleInContainer(Exception):
     def __reduce__(self):
         return self.__class__, tuple(getattr(self, f.name) for f in fields(self))

-@dataclass(frozen=False)
+@dataclass(frozen=True)
 class UndefinedBehavior(UserWarning, PintError):
     msg: str
bryanwweber commented 4 months ago

I've also run into this today. I applied @simonw's suggested change and the tests all passed. Is that the appropriate fix?

bje- commented 4 months ago

Can we please get this fixed so that I can start testing with Python 3.13 before it is released?

simonw commented 3 months ago

In trying to ship a PR for this I found the following problem:

Document: getting/tutorial
--------------------------
**********************************************************************
File "getting/tutorial.rst", line 78, in default
Failed example:
    speed = 23 * ureg.snail_speed
Expected:
    Traceback (most recent call last):
    ...
    UndefinedUnitError: 'snail_speed' is not defined in the unit registry
Got:
    Traceback (most recent call last):
      File "/opt/hostedtoolcache/Python/3.10.14/x64/lib/python3.10/doctest.py", line 1350, in __run
        exec(compile(example.source, filename, "single",
      File "<doctest default[0]>", line 1, in <module>
        speed = 23 * ureg.snail_speed
      File "<string>", line 4, in __setattr__
    dataclasses.FrozenInstanceError: cannot assign to field 'name'
**********************************************************************

I'm surprised that only showed up in doctest and not in the rest of the unit test suite, but it helps me understand that there's a reason for the frozen=False argument used here.

simonw commented 3 months ago

I've stopped working on this - I hope someone else can figure it out.

bje- commented 1 month ago

@simonw Your change is going to be problematic because it will revert 08b9807e5e197b7b94bcad00585f0aaa3b5fe19e by @hgrecco. Hernan, can you elaborate on why you made this change in the first place?

bje- commented 1 month ago

@simonw Your change is going to be problematic because it will revert 08b9807 by @hgrecco. Hernan, can you elaborate on why you made this change in the first place?

Ah, I've just read #1766 so now understand.

simonw commented 3 weeks ago

Python 3.13 was released today - https://docs.python.org/3.13/whatsnew/3.13.html - any chance of a compatible Pint release?

simonw commented 3 weeks ago

The root cause here appears to be this bug fix in Python:

The dataclass inheritance hierarchy is supposed to require all classes to be either frozen or non frozen, this works properly for checking that an unfrozen class does not inherit from any frozen classes, but it allows frozen classes to inherit from unfrozen ones as long as there's at least one frozen class in the MI

Fixing that bug exposed that Pint was relying on the buggy behaviour.

Erotemic commented 2 weeks ago

When is this fix planned to be pushed to pypi? I'm unable to test my software stack in 3.13 without this library.

bje- commented 2 weeks ago

When is this fix planned to be pushed to pypi? I'm unable to test my software stack in 3.13 without this library.

Likewise. Maybe a fork is needed in the meantime?

hgrecco commented 2 weeks ago

Sorry for the long delay. This will be fixed by next week and a minor release will be produced