PyO3 / pyo3

Rust bindings for the Python interpreter
https://pyo3.rs
Apache License 2.0
12.49k stars 770 forks source link

Docstrings for `#[new]` method implementations #4326

Open davidhewitt opened 4 months ago

davidhewitt commented 4 months ago

Discussed in https://github.com/PyO3/pyo3/discussions/4280

Originally posted by **djc** June 23, 2024 When I write a docstring on a `#[new] fn new()` method in a `#[pymethods]` `impl` block, I would expect this to show up in the `help()` output for the type. Instead, the `help()` output seems to end up with a default docstring: ``` | Static methods defined here: | | __new__(*args, **kwargs) | Create and return a new object. See help(type) for accurate signature. ``` Is there no way in Python's data model to enable a specific help string for this? The docstring for the type ends up making it into the output sort of next to the constructor type signature, so I guess I could use that. Just seems surprising.

I believe that it should be possible to do something similar to what we already do with the text_signature for #[new] to lift the rest of the documentation onto the class. But this needs some experimentation.

djc commented 4 months ago

If you can give me a few pointers, I might be able to take a look myself.

davidhewitt commented 4 months ago

So changing the exact message Create and return a new object. See help(type) for accurate signature. is probably not possible. These all come from the type "slot wrappers" which take just a function pointer to the C slot implementation and then use that to generate a member on the class.

The one place where we can add class-level documentation is in the Py_tp_doc slot.

We already use pyo3::impl_::class::PyTypeImpl::doc to add the "text signature" from #[new] into the class-level documentation. We could similarly lift docs from #[new]. I think the only question is where in the class level documentation we should apply the docs from #[new], if adding this in is a good idea.

jfolz commented 4 months ago

Does this also affect other magic methods like __lt__? Regardless of what I write on the Rust side, its docstring is always "Return self<value."

davidhewitt commented 4 months ago

Yes, the same is true of these. I think the Python "slots" don't allow for documentation to be added, we would need to ask the CPython team for this.

davidhewitt commented 4 months ago

That said, there is METH_COEXIST which would allow us to install our own Python-level __lt__ (which could then have documentation). This would be a change to the proc-macros but might be a good thing. 🤔

jfolz commented 4 months ago

TBF though, I just check what help(set.__lt__) says:

__lt__(self, value, /) unbound builtins.set method
    Return self<value.

If set doesn't care to overwrite its docstring with a more sensible explanation, why should I care? :D

djc commented 2 months ago

Looking at this with fresh eyes, I think PyO3 is already doing what needs to be done, and not sure there is anything for me to be done here.

ngoldbaum commented 1 month ago

I think what's missing is setting the doc field in the PyMemberDef struct for each method. Looking at the code that sets up the PyMemberDef structs in create_type_object.rs (I think that's the correct spot), it doesn't look like the doc field is ever set.

Walter-Reactor commented 1 month ago

+1 to this, I came here to report this issue. I've got constructors that would greatly benefit from having docstrings