Skip to content
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

Automatically implement Python getters and setters for public Rust struct fields #1375

Open
kangalio opened this issue Jan 10, 2021 · 9 comments

Comments

@kangalio
Copy link
Contributor

This would make Rust<->Python intercommunication even more seamless. In Rust, you can access struct fields if they are public - why not in Python?

I can't see any problems if PyO3 automatically made Rust struct fields accessible when appropriate, but I'm happy to learn if there are issues I hadn't considered.

@kangalio
Copy link
Contributor Author

Currently, the best design I can think of is to check:

  • Do any of the struct's pub fields have a #[pyo3(get)] or #[pyo3(set)] attribute?
    • If yes, apply those, just like in earlier PyO3 versions
      • If all fields are marked both #[pyo3(get)] and #[pyo3(set)] (making the behavior identical to the fallback implementation), emit a compiler warning that you can remove all those annotations
    • If no, automatically implement getters and setters for all the pub fields

This approach has the benefit that existing structs don't mysteriously change behavior, and new plain-old-data structs can be created painlessly without having to annotate every field.

Its disadvantage is that this behavior may be slightly confusing to end users: all fields are accessible by default, but if you add a #[pyo3(get)], suddenly all other fields are not accessible from Python anymore?

Does someone have an idea to improve upon this design, or is there a better design idea altogether?

@birkenfeld
Copy link
Member

birkenfeld commented Jan 21, 2021

Why does it have to be this implicit? A single new attribute or argument (on the struct, not the members) isn't too much to write, and makes it explicit that accessors are automatically generated.

I.e.

#[pyclass(pod)]
struct Pod {
    x: i32,
    y: i32
}

or

#[pyclass]
#[pyo3(get_set_all)]

Bikesheds welcome...

@davidhewitt
Copy link
Member

👍 I am strongly in favour of making this an explicit opt-in; doing things automatically is not very idiomatic in Rust. I think it can be the same attribute as for #1376.

#[pyclass(pod)] or #[pyclass(dataclass)] is quite reasonable as a suggestion. It should require all fields are public and could automatically provide #[new] as well as getters and setters for all the fields.

@kangalio
Copy link
Contributor Author

kangalio commented Jan 24, 2021

In that case, I think #[pyclass(dataclass)] is preferable over #[pyclass(pod)] or #[pyclass(get_set_all)]. "pod" is potentially confusing for people who don't know the term. "get_set_all" is explicit, but it can't be used as the same attribute for #1376. "dataclass" references an existing well known Python concept and concisely encapsulated the behavior both from this feature and from #1376.

@davidhewitt
Copy link
Member

We may want to also consider the existence of such an attribute when thinking about design for #[pyenum] we have proposed for #1045, and #[pyexception] in #295 . Should they become #[pyclass(enum)] and #[pyclass(exception)] for consistency?

@davidhewitt
Copy link
Member

In that case, I think #[pyclass(dataclass)] is preferable over #[pyclass(pod)]

Note that @dataclass in Python also adds hash, repr and ordering - so we may want to provide an implementation of PyObjectProtocol also, to match Python as best as possible.

@igiloh-pinecone
Copy link

Are there any new on this issue? Any plans to implement #[pyclass(dataclass)] in the coming future?

@mejrs
Copy link
Member

mejrs commented Jan 30, 2023

Are there any new on this issue? Any plans to implement #[pyclass(dataclass)] in the coming future?

We did add [pyclass(get_all, set_all)] in 0.18, so if all you want is to generate getters and setters for all fields you can use that. Also, frozen exists, though I am not sure it is an exact match to a dataclass' frozen.

As for other features associated with "data classes", like constructors, dunder method implementations etc I don't think anyone is working on that. It would be up to someone to come up with a design and an implementation.

@igiloh-pinecone
Copy link

We did add [pyclass(get_all, set_all)] in 0.18, so if all you want is to generate getters and setters for all fields you can use that

I think one of the main asks for a #[pyclass(dataclass)] was to remove the need for an explicit #[new], which still won't be achieved with this solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants