Implement `auto_new` attribute for `#[pyclass]`
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
I am not quite sure why the CI failing, I dont see any failure in the logs
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.
auto_newis maybe the right name, the only alternative I know is something likedataclass
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.
@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
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__.
With #5551 merged, this should be able to rebase on top now.
I am running into an issue, I think generate_protocol_slot assumes first argument is self, and generates broken code when it isn't. Image below shows cargo expanded version of this simple test struct.
#[pyclass(get_all, set_all, new = "from_fields")]
struct AutoNewCls {
a: i32,
b: String,
c: Option<f64>,
}
I am not sure how to go about fixing this properly @davidhewitt maybe you could take a look? I pushed the latest broken commit with it, other than that it should work but hard to tell with this being broken.
looks like nox -s update-ui-tests isnt working for me :/ I think I am running on a different rust version producing slightly different errors, as it slightly modifies unrelated tests as well
I pushed a commit which should fix. The &str extraction only works like that on Python 3.9 and older, perhaps caused by your system Python? I am unsure why we don't notice that as a problem in our CI 🤔
We use latest stable Rust, also latest trybuild version (rustup update stable && cargo update should align you with CI).
I will give another pass at review probably on Sunday.
Looks like the &str case in the UI test is indeed still a problem. I'm currently investigating that code to see what we can do to improve the error message.
Circling back here, I have some ideas how to solve that problem, but the async code being heavily special cased (xref #5681) is causing issues for me. If I get that code adjusted to not have the special case I think I can land cleanups to unblock here.