-
-
Notifications
You must be signed in to change notification settings - Fork 374
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
Add optional namespace arguments for make_class
#1203
Conversation
`make_class` Signed-off-by: Aaron <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron <29749331+aarnphm@users.noreply.github.com>
make_class
make_class
make_class
make_class
make_class
make_class
I think this is an interesting idea as per our ideal to be a class construction kit! But I don't think namespaces is a good term for what it does. Wanna brainstorm something more obvious? Maybe |
I think class_body makes sense. My guess that from a Python developer perspective, this would infer that attributes here are class attributes. Though I'm not quite sure if users implement a
Oh I forgot to mention that for type inference, mypy plugin for |
Signed-off-by: Aaron <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron <29749331+aarnphm@users.noreply.github.com>
I mean… they are? Am I misunderstanding what your code is doing? 😅
Can you elaborate what you mean by that please?
That sounds like a @Tinche problem. 😇 |
Oh, I think we are aligned on this.
Yes, so for the following example import attr
@attr.define
class Base:
x: int
y: t.List[int] = attr.field()
def __class_getitem_override(cls, item): # what is the type of item here?
if item.__name__ == 'y': return [1,2,3]
else: return cls.__getitem__(item)
new_class = attr.make_class("NewClass", bases=(Base,), namespaces={"__class_getitem__": __class_getitem_override}) Users cannot be prevented from doing something like this, right? What should be the type of |
Oh god TIL about |
This is the one concern I have when thinking about this feature, so probably will def need a discussion about this. |
OK since this seems to be a somewhat stalling issue: is your PR in any way special to From reading the docs around it, I understand it's a fallback if there's no Like we try hard to not create attractive nuisances, but we also try to not be too limiting. |
Ye I think it should be fine. So that is probably not an issue. This one is ready to merge. |
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.
Thanks!
Signed-off-by: Aaron 29749331+aarnphm@users.noreply.github.com
Summary
Add an optional
namespaces
argument toattr.make_class
to allow users to inject addtional namespaces attributes into newly created attrs class.I think this is pretty useful for cases where a derivation of the base class is needed with additional functionalities. Since
make_class
usetypes.new_class
under the hood this is possible.Currently, if users want to derivate from a attrs classes, one might do:
This will lose types information and therefore users might need to provide additional stubs for the new class. With this PR, users can do:
Which is nice, but might not be a good idea.
Pull Request Check List
main
branch – use a separate branch!Our CI fails if coverage is not 100%.
.pyi
).tests/typing_example.py
.attr/__init__.pyi
, they've also been re-imported inattrs/__init__.pyi
.docs/api.rst
by hand.@attr.s()
have to be added by hand too.versionadded
,versionchanged
, ordeprecated
directives.The next version is the second number in the current release + 1.
The first number represents the current year.
So if the current version on PyPI is 22.2.0, the next version is gonna be 22.3.0.
If the next version is the first in the new year, it'll be 23.1.0.
.rst
and.md
files is written using semantic newlines.changelog.d
.