-
Notifications
You must be signed in to change notification settings - Fork 258
non-final frozen attributes (was: Improved read-only attributes of Protocol) #922
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
Comments
☝️ NOTEI have since refined this issue, see #922 (comment) Below I have kept the original text One usecase that complicates things is adding mutability through inheritance: class Address(Protocol):
street: Final[str] # only readable
zipcode: Final[int]
class MutableAddress(Address):
street: str # readable and writable
zipcode: int What are your thoughts on this? If we want to support this,
|
It seems the problem is more general: could we have a dedicated way to prevent reassignment of attributes without also preventing overriding (which The problemThe goals of
The problem is that (1) limits (3). Consider the following: class Measurement:
timestamp: Final[date] # final used here to prevent reassigning this attribute
value: int
def __init__(self, value: int):
self.timestamp = date.today()
self.value = value
m = Measurement(0)
m.timestamp = date(2020, 1, 1) # this is prevented, as it should be
class AccurateMeasurement(Measurement):
# Declaring this attribute is currently not allowed because Final forbids overriding `timestamp`.
# If we're only interested in preventing attribute reassignment, it should be possible though.
timestamp: Final[datetime] # (note: datetime is a subclass of date)
def __init__(self, value: int):
self.timestamp = datetime.now()
self.value = value ProposalAn annotation specifically for preventing reassignment of attributes, but that does not prevent overriding. An additional advantage is that readonly members of protocols become way easier to define: class MeasurementLike(Protocol):
timestamp: Frozen[date] instead of class MeasurementLike(Protocol):
@property
def timestamp(self) -> date:
... |
Well, technically you override class AccurateMeasurement(Measurement):
# Declaring this attribute is currently not allowed because Final forbids overriding `timestamp`
# If we're only interested in preventing attribute reassignment, it should be possible though.
timestamp: Final[datetime] # note: datetime is a subclass of date
def __init__(self, value: int):
self.timestamp = datetime.now()
self.value = value It is just made via subclassing that reassignes This will be especially visiable when |
@sobolevn you could indeed interpret it that way 🤔. Maybe it's better to talk of 'frozen' attributes, like with dataclasses: (the following code checks out) @dataclass(frozen=True)
class Measurement:
value: int
timestamp: date = field(default_factory=date.today, init=False)
@dataclass(frozen=True)
class AccurateMeasurement(Measurement):
timestamp: datetime = field(default_factory=datetime.now, init=False)
m = Measurement(1)
a: Measurement = AccurateMeasurement(2) it'd be useful to express 'frozenness' of attributes outside of dataclasses: class Measurement:
value: Frozen[int]
timestamp: Frozen[date]
class AccurateMeasurement(Measurement):
timestamp: Frozen[datetime] |
Note that PEP591 does seem to imply 'reassignment' of attributes means: assignment outside In any case, the rule for |
additionally, a class Measurement:
value: Frozen[int]
timestamp: Frozen[date]
# equivalent to
@frozen
class Measurement:
value: int
timestamp: date A lot more readable as the number of attributes increases! |
Found a related discussion about read-only protocol members: #903 (conclusion was use the |
@ilevkivskyi I was wondering if you had any thoughts on the matter -- since you co-authored PEP591 and wrote the reference implementation for |
IMO the convenience of saving one line of code (by not using a property) is not worth introducing some special rules/concepts. IIRC final things were developed explicitly to not overlap with existing solutions (like properties). |
What use case isn't covered by |
Thanks @srittau @ilevkivskyi for the remarks! I can definitely understand not wanting to add To answer your question, I'd say there are 2 limitations of Limitation 1: properties in Protocols and ABCs are so verbose that they obscure intentionclass MeasurementLike(Protocol):
@property
def timestamp(self) -> date:
...
class MeasurementBase(ABC):
@property
@abstractmethod
def timestamp(self) -> date:
...
# compare with the much less noisy
class MeasurementLike(Protocol):
timestamp: Final[date] Does this occur often enough to warrant a shorthand? Potential solution to limitation 1: a type qualifier for propertyA simple solution would be a class MeasurementLike(Protocol):
timestamp: Property[date]
class MeasurementBase(ABC):
timestamp: Property[date] # no implementation means it's abstract We could also simply allow Limitation 2: In concrete classes,
|
FYI I created a small mypy plugin for fixing the initial problem of read-only protocols: link Nevertheless I'd be interested to hear any thoughts on the |
I see that this particular I will proceed to close this issue 🔒 |
☝️ NOTE
I have since refined this issue, see #922 (comment)
Below I have kept the original text
Are there any ideas about making read-only attributes in
Protocol
easier to define? Immutable objects (e.g.datetime
) and frozen dataclasses are great, but they are painful to integrate withProtocol
. Which is a shame, because immutability and structural typing are both great!The problem
Why do something about it
In my experience, the use case for purely read-only attributes is far more common than settable attributes. It feels strange that this requires such a more cumbersome and less readable workaround. This can be easily fixed without breaking existing behavior.
Proposed solution: allow using
Final
or another annotationCurrently this gives an explicit error (
error: Protocol member cannot be final
). Understandable, because the semantics don't quite match up.However, PEP591 does seem to leave the door open a bit:
An alternative would be a separate
ReadOnly[...]
annotation or similarWhat are your thoughts?
(possibly related: #920)
The text was updated successfully, but these errors were encountered: