Skip to content

gh-118974: Add decorator argument to make_dataclass #122723

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

Merged
merged 6 commits into from
Oct 1, 2024

Conversation

Viicos
Copy link
Contributor

@Viicos Viicos commented Aug 6, 2024

@Viicos Viicos marked this pull request as ready for review September 1, 2024 10:23
@Viicos Viicos requested a review from ericvsmith as a code owner September 1, 2024 10:23
@ericvsmith
Copy link
Member

I'm not crazy about the name dataclass_factory. What do you think about dataclass_decorator?

@ericvsmith
Copy link
Member

Other suggestions at the core sprint are prepare and decorator.

@Viicos
Copy link
Contributor Author

Viicos commented Sep 29, 2024

I'm not crazy about the name dataclass_factory. What do you think about dataclass_decorator?

I think dataclass_decorator makes more sense if @dataclass() is only meant (or at least documented) to be used as a decorator, and not as a "factory" (i.e. MyClass = dataclass(...)).

But anyway, I'm happy to make the change to use dataclass_decorator. Let me know if you want me to push the necessary changes.

@ericvsmith
Copy link
Member

@larryhastings : Any thoughts on the parameter name? I'm leaning towards just decorator. I think everyone knows dataclass() as a decorator. dataclass_decorator seems redundant: decorator=dataclass looks reasonable to me: The decorator you'd call if you were statically defining a class.

@larryhastings
Copy link
Contributor

I think decorator is an excellent name here. Here's some snippets from the definition of decorator from the official Python glossary:

A function returning another function, usually applied as a function transformation using the @wrapper syntax.
Common examples for decorators are classmethod() and staticmethod().

The decorator syntax is merely syntactic sugar, [...] The same concept exists for classes, but is less commonly used there. [...]

That strikes me as an excellent fit. make_dataclass is calling this function to transform a class into another class, and decorators are transformers returning a different object of the same type. I worried the term decorator might specifically refer to "the application of the @-sign syntax", but the glossary makes it clear that's not the focus.

Also, I prefer decorator to factory here because I think of a factory as making a new and different thing out of raw materials. In my mind, a "car factory" ingests sheet metal and bolts and aluminum and paint and fabric and produces a car; it doesn't transform an existing car into a different kind of car. But this callable is transforming one class into another class. So the word decorator is a better fit. (Also factory sounds really Java-y, and who wants that association in the Python world!) I concede this might be a personal taste thing, and other folks wouldn't have these associations... but you did ask my opinion.

I think I'm the dude who suggested preparer at the sprint. It was a straight-off-the-dome suggestion made with little context or consideration. I thought of preparer because of the __prepare__ magic method on metaclasses used during class creation, which I suggest is vaguely analogous. So I think preparer is okay--but decorator is much better!

@Viicos
Copy link
Contributor Author

Viicos commented Sep 30, 2024

(Also factory sounds really Java-y, and who wants that association in the Python world!)

Convinced! Just pushed a commit fixing this.

@ericvsmith
Copy link
Member

Thanks, @larryhastings for the excellent analysis (I expected nothing less!). I'll review the patch when I have some free time.

@ericvsmith ericvsmith self-assigned this Sep 30, 2024
@Viicos Viicos changed the title gh-118974: Add dataclass_factory argument to make_dataclass gh-118974: Add decorator argument to make_dataclass Oct 1, 2024
@ericvsmith ericvsmith merged commit 3e3a4d2 into python:main Oct 1, 2024
34 checks passed
@Viicos Viicos deleted the fix-issue-118974 branch October 1, 2024 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants