Skip to content

Conversation

@RedKinda
Copy link

@RedKinda RedKinda commented Sep 8, 2025

PR for issue #5416

This introduces a new #[pyclass] attribute called auto_new, where set_all is required to be used alongside it. It automatically generates a #[new] function for this class, similar to that of the python dataclass. It requires the multiple-pymethods feautre, as it generates its own #[pymethods] block

@RedKinda
Copy link
Author

RedKinda commented Sep 8, 2025

I am not quite sure why the CI failing, I dont see any failure in the logs

@RedKinda RedKinda changed the title Dev/auto new Implement auto_new attribute for #[pyclass] Sep 8, 2025
Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, and sorry for the very slow review.

I think this feature makes a ton of sense, I've wanted something like this for ages. auto_new is maybe the right name, the only alternative I know is something like dataclass where this is roughly equivalent to the dataclass(init=True) option. I don't think init is correct here though, since this generates __new__.

To resolve before merge: #5284 proposes #[pyclass(default)] which would allow a constructor which takes no arguments and uses Default::default(). It might be nice to not have separate default and auto_new attributes. What if we had #[pyclass(auto_new = "from_fields")] (this PR) and #[pyclass(auto_new = "default")] ? Name and design ideas welcome.

Other open questions, potentially fine to leave for follow ups:

  • We could allow #[pyo3(default = X)] annotations on the fields to construct them.
  • We could consider #[pyo3(kw_only)] at both the class and field level.

The CI failure is UI tests, you can fix with nox -s update-ui-tests:

https://github.com/PyO3/pyo3/actions/runs/17557400301/job/49865008001?pr=5421#step:19:3264

Comment on lines +2235 to +2268
if matches!(methods_type, PyClassMethodsType::Specialization) {
bail_spanned!(opt.span() => "`auto_new` requires the `multiple-pymethods` feature.");
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need this restriction (at the cost of a slightly more complicated implementation) - see generate_protocol_slot and how it's used for __str__.

@RedKinda
Copy link
Author

Hi, thanks for the review!

In regards to the defaults-related PR, #[pyclass(auto_new = "from_fields")] and #[pyclass(auto_new = "default")] make sense to me, names sounds fine.

As for the followups, I would leave those to be in a separate PR and get this and the default one out first. I don't have the capacity to work on them at the moment but maybe if we open an issue someone will pick it up. I am also gonna look at the generate_protocol_slot comment before merging, but might take a bit since I am not familiar with it yet.

@Icxolu
Copy link
Contributor

Icxolu commented Oct 21, 2025

auto_new is maybe the right name, the only alternative I know is something like dataclass

Perhaps we could just call it new. The "auto" feels a bit redundant to me. Just setting the option already implies that something is happening. We don't call it auto_str for example, just str.

@RedKinda
Copy link
Author

@davidhewitt in regards to generate_protocol_slot for auto_new should that be a new pub const __NEW__: SlotDef? not sure what else but just confirming

@Icxolu i think it might be a little confusing for newbies reading docs to have both #[pyclass(new)] and #[new] on functions, but i don't feel strongly about this

@davidhewitt
Copy link
Member

Good question, #5551 produces a __NEW__ slotdef which you should be able to use, I'm going to split that diff up. Feel free to either base off that PR or give me some time to merge it in stages (might take a few days).

I think #[pyclass(new)] seems correct to me, this matches #[pyclass(str)] and __str__. I would quite like to have #[new] optionally just be __new__.

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.

3 participants