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

[simonw]

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]

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]

It's possible this issue is related:

• gh-109409: Fix inheritance of frozen dataclass from non-frozen dataclass mixins python/cpython#109437

See cPython commit: python/cpython@b6000d2

[simonw]

I think this is the code that the trackback complains about:

pint/pint/delegates/txt_defparser/common.py

Lines 15 to 25 in f2e4081

	from dataclasses import dataclass, field
	import flexparser as fp
	from ... import errors
	from ..base_defparser import ParserConfig
	@dataclass(frozen=True)
	class DefinitionSyntaxError(errors.DefinitionSyntaxError, fp.ParsingError):
	"""A syntax error was found in a definition. Combines:

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

pint/pint/errors.py

Lines 105 to 115 in f2e4081

	@dataclass(frozen=False)
	class DefinitionSyntaxError(ValueError, PintError):
	"""Raised when a textual definition has a syntax error."""
	msg: str
	def __str__(self):
	return self.msg
	def __reduce__(self):
	return self.__class__, tuple(getattr(self, f.name) for f in fields(self))

[simonw]

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]

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

[bje-]

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

[simonw]

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]

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

[bje-]

@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?

[bje-]

@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]

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

[simonw]

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

• frozen dataclass inheritance is not strictly checked in multiple inheritance python/cpython#109409

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.

[mraspaud]

I can confirm pinning flexparser<0.4 works in our CI

[ndevenish]

Should pint have upper flexparser e.g. flexparser>=0.4,<0.5?

[Kattemageren]

Should pint have upper flexparser e.g. flexparser>=0.4,<0.5?

Literally read the previous reply

[epenet]

Should pint have upper flexparser e.g. flexparser>=0.4,<0.5?

Literally read the previous reply

I think the point is that pint should already have been using an upper constraint on flexparser.
If pint had been using an upper constaint previously, the impact would have been greatly reduced...

[ndevenish]

I think the point is that pint should already have been using an upper constraint on flexparser. If pint had been using an upper constaint previously, the impact would have been greatly reduced...

I mean, it would have avoided this issue here, but my point isn’t that it should have been restricted, and more that going forward, should it now be restricted, so it doesn’t happen again?

[hgrecco]

I apologize for the inconvenience. In relation to upper version limits, I think the comment here also makes sense:

hgrecco/flexparser#12 (comment)

While I am open for discussion, I think this type of changes will not happen very often. This change was needed due to a modification in Python 3.13 of the semantics of frozen dataclasses. Dataclasses appeared in Python 3.7, 6 years ago, and I considered this stable enough to be used in something as fundamental as exceptions. This was not the case and came back to bite me!.

Anyway, I will release a bug fix of Pint and we tackle this in another issue.

[keewis]

While I am open for discussion

Upper bounds on a library's dependencies are usually not a good idea, so I'd be -1 on that: without the upper bound people can do something about the breaking change by pinning flexparser, but issues caused by upper bounds means that we can't do anything other than waiting for a new release.

So I'd much rather follow the advice quoted in the TL;DR of the article I linked to:

Libraries/packages should be setting a floor, and if necessary excluding known buggy versions, but otherwise don’t cap the maximum version as you can’t predict future compatibility

[hgrecco]

I just pushed Pint 0.24.4. If everything is fine, It will be in PyPI shortly.

[hgrecco]

I am closing this issue as is fixed in Pint 0.24.4 Please let me know if you encounter any problem.

[ruishopily]

Thanks so much for the quick fix! It works.

[ndevenish]

Upper bounds on a library's dependencies are usually not a good idea, so I'd be -1 on that: without the upper bound people can do something about the breaking change by pinning flexparser, but issues caused by upper bounds means that we can't do anything other than waiting for a new release.

Generally my rule of thumb for upper bounds are:

• Packages that have exhibited "Strong Semver" in the past e.g. actually breaking changes
• Packages that are tightly coupled (e.g. by the same author/group, one is the backend for the other)
• Packages that have caused me issues

This case hits all three. The extremely over-long article you posted actually agrees with this evaluation btw, calling it the "best" reason to cap:

You are releasing two or more libraries in sync with each other. You control the release cadence for both libraries. This is likely the “best” reason to cap. Some of the above issues don’t apply in this case - since you control the release cadence and can keep them in sync.

[ndevenish]

And the update has hit conda-forge now also: conda-forge/pint-feedstock#73

So I think all is good. Thank you for fixing!