-
-
Notifications
You must be signed in to change notification settings - Fork 395
Expose effective class construction params #1454
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
base: main
Are you sure you want to change the base?
Conversation
2541864
to
af0f118
Compare
470e2eb
to
1edde23
Compare
src/attr/_make.py
Outdated
is_slotted: bool | ||
has_weakref_slot: bool | ||
is_frozen: bool | ||
is_kw_only: bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we combine these flags? We can't have force_kw_only without is_kw_only right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that is correct… so another Enum?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I use literals for this in cattrs but that might be too modern for your tastes ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might want to give hash
the same treatment - cache_hash
doesn't make sense if hash
is false. So the enum might be expanded with HASHABLE_CACHED
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
both done, but we should probably come up with more consistent naming.
Hashable.YES
, YES_CACHED
, NO
, LEAVE_ALONE
?
match_args: bool | ||
str: bool | ||
getstate_setstate: bool | ||
on_setattr: Callable[[str, Any], Any] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should these Callables be optional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it ever none? looking at the docs:
If left None, the default behavior is to run converters and validators whenever an attribute is set.
I think it's set to NO_OP if nothing is supposed to happen?
d091898
to
9575cf9
Compare
This reflects that the instance is NOT just the parameters as passed to the decorator, but how the class is actually constructed.
And make Hashability public
@Tinche what did we agree to call the inspect function that extracts the props? |
ref #602
This is for now just private and the impact zone is limited. But I'm confident it can help us to simplify a bunch of code.
I'm especially rather unhappy with the weird entanglement of _ClassBuilder that doesn't really have a clear purpose (anymore?).
But let's talk in general first!