Introspection: modules associated constants

[yogevm15]

No description provided.

[yogevm15]

How can i add labels? (to skip the changelog check)

[Tpt]

Thank you!

For the labels, I think you need some rights on this repo to do that.

On the MR itself:

• I would maybe implement constants as an other stand-alone "member", just like functions/classes/... This way if constants are defined outside a module then imported into it (like classes or functions) we don't have to change the introspection data structure.
• For simple constants (strings, integers...) it would be great to follow the stub guides and to output PI: Final = 3.14. There is already some logic to convert Rust values to Python, we might reuse it there (I already used it to generate the function signatures). This is a refinement so feel free to just not do it or postpone it as a follow up. Just having the constant names in the type stubs is already great!

[davidhewitt]

Actually I think we probably do want changelog entries for these additions now, as the original version of this introspection shipped in 0.25. It would be good to document improvements to it as future releases come.

[yogevm15]

I would maybe implement constants as an other stand-alone "member", just like functions/classes/... This way if constants are defined outside a module then imported into it (like classes or functions) we don't have to change the introspection data structure.

Currently, constants defined outside a module and then re-exported are not supported.
To support this use case, we would need to add a pyconst macro. I'm not sure it's worth it, since one can simply declare a new constant within the module that equals the external constant.

If we eventually decide to go with the pyconst macro, I think we can just add support for standalone constants member and have both consts list and standalone constants member.

What do you think?

[Tpt]

If we eventually decide to go with the pyconst macro, I think we can just add support for standalone constants member and have both consts list and standalone constants member.

Indeed. However, I would feel better to have an as open to evolution introspection data format as possible: the pyo3-introspection crate will be a dependency of eg. Maturin so we having forward and backward compatibility as much as possible would be amazing.

I believe we can always add typing.Final to const elements, they are always immutable.

I am also a bit scared about #[final_var("3.14")] syntax because it's an easy footgun: one can update the actual constant value and forget about the value in the macro it's likely no automated tool will flag anything.

What about having a dumb but easy behavior like:

const PI:  f64 = std::f64::consts::PI;

emits:

PI: typing.Final = ...

and in a follow up MR:

PI: typing.Final[float] = ...

and

const PI:  f64 = 3.14;

emit:

PI: typing.Final = 3.14

What do you think?

In this case, having a pyconst macro might be handy to allow declaring constants outside of the #[pymodule] while still having the explicit value in the stubs (but I am not sure it's very useful outside of having the actual value displayed in IDEs).

[yogevm15]

Indeed. However, I would feel better to have an as open to evolution introspection data format as possible: the pyo3-introspection crate will be a dependency of eg. Maturin so we having forward and backward compatibility as much as possible would be amazing.

I think it's a bit odd to implement it this way without the pyconst macro, I'm open to implement the macro in this PR if you think that's the right approach.

I believe we can always add typing.Final to const elements, they are always immutable.

You're right!

I am also a bit scared about #[final_var("3.14")] syntax because it's an easy footgun: one can update the actual constant value and forget about the value in the macro it's likely no automated tool will flag anything.

On second thought, I agree with you.

[Tpt]

Thank you!

I think it's a bit odd to implement it this way without the pyconst macro, I'm open to implement the macro in this PR if you think that's the right approach.

I tend to think it's out of scope of this PR. But I would love to see it in a follow-up if PyO3 maintainers agree.

This MR looks good to me, but I do not have merge right on PyO3.

[Icxolu]

Found a small typo, otherwise looks good to me as well (with my still limited insight into the introspection code 🙃) Thanks @yogevm15 for the implementation and @Tpt for the reviewing.