make `GILOnceCell::get_or_try_init_type_ref` public

PyO3 / pyo3

Rust bindings for the Python interpreter

https://pyo3.rs

Apache License 2.0

12.39k stars 765 forks source link

make `GILOnceCell::get_or_try_init_type_ref` public #4542

Closed glevco closed 1 month ago

glevco commented 2 months ago

Resolves https://github.com/PyO3/pyo3/issues/4516.

Change GILOnceCell::get_or_try_init_type_ref to GILOnceCell::import and make it public API, adding an example for it.
Update "Executing existing Python code" section of the guide:
- Add mention to GILOnceCell::import.
- Change Python::eval_bound (which is deprecated) to Python::eval.
- Change Python::run_bound (which is deprecated) to Python::run, also adding missing link.

glevco commented 2 months ago

@mejrs

Why? I don't have a strong opinion either way, but slightly prefer PyModule::import.

Since Python::import was used in the implementation of get_or_try_init_type_ref, I assumed it was preferable. Also, it pairs nicely with the other entries in this section of the guide, as they recommend other "global" functions such as Python::eval and Python::run. What do you think?

davidhewitt commented 2 months ago

I prefer PyModule::import too, I think it's nicer to think of that as a "constructor" for the PyModule type.

mejrs commented 2 months ago

What do you think?

From a documentation perspective I would prefer that people visit PyModule::import over Python::import. Python has a lot of not module related api, and its page is a lot bigger.

Also, using "constructor" syntax also makes it clear that when they use it, they're getting a module (rather than something arbitrary like from foo import bar might do).

glevco commented 2 months ago

@davidhewitt @mejrs Fair enough, I reverted the guide back to PyModule::import in https://github.com/PyO3/pyo3/pull/4542/commits/79568f6c267d8973504eb94b19ae0f32a92d066c. I also changed the method name to GILOnceCell::import and made its impl generic for PyTypeCheck, as discussed above. I updated the PR description accordingly.