Expand on AsPyPointer docs and un-ignore doc examples #2350
Conversation
mejrs
commented
May 2, 2022
mejrs
commented
- Expands on the AsPyPointer/IntoPyPointer docs to warn about Allow more methods to take interned arguments #2312 (comment)
- Un-ignores some doctests
guide/src/class.md
Outdated
| Note that unlike class variables defined in Python code, class attributes defined in Rust cannot | ||
| be mutated at all: | ||
| ```rust,ignore | ||
| // Would raise a `TypeError: can't set attributes of built-in/extension type 'MyClass'` | ||
| pyo3::py_run!(py, my_class, "my_class.my_attribute = 'foo'") | ||
| ```rust,should_panic | ||
| # use pyo3::prelude::*; | ||
| # #[pyclass] | ||
| # struct MyClass {} | ||
| # #[pymethods] | ||
| # impl MyClass { | ||
| # #[classattr] | ||
| # fn my_attribute() -> String { | ||
| # "hello".to_string() | ||
| # } | ||
| # } | ||
| # | ||
| Python::with_gil(|py| { | ||
| let my_class = py.get_type::<MyClass>(); | ||
|
|
||
| // Would raise a `TypeError: can't set attributes of built-in/extension type 'MyClass'` | ||
| pyo3::py_run!(py, my_class, "my_class.my_attribute = 'foo'") | ||
| }); |
There was a problem hiding this comment.
As per #2349 shall we just change the documentation here? I'm not sure if it's worth us investing lots of effort to change this, or how hard it would be.
| @@ -311,7 +311,7 @@ from anywhere as long as your `app.py` is in the expected directory (in this exa | |||
| that directory is `/usr/share/python_app`). | |||
|
|
|||
| `src/main.rs`: | |||
| ```ignore | |||
| ```no_run | |||
There was a problem hiding this comment.
| ```no_run | |
| ```rust,no_run |
... maybe?
There was a problem hiding this comment.
I decided against running it because it loads things in a very platform dependent way.
There was a problem hiding this comment.
I was just wondering whether the rust tag was also needed to give nice syntax highlighting.
There was a problem hiding this comment.
Oh right. For some reason I glanced over that it would still be no_run
| @@ -722,7 +722,7 @@ If `T` implements `Clone`, you can extract `T` itself. | |||
| In addition, you can also extract `&PyCell<T>`, though you rarely need it. | |||
|
|
|||
| Before: | |||
| ```ignore | |||
| ```compile_fail | |||
There was a problem hiding this comment.
I'm not sure it's worth changing bits like this which we know have old syntax, it's presumably just wasting CPU cycles to assert that the compile fails. Is there a particular intended benefit to this change?
There was a problem hiding this comment.
Well, I'd like to see to it that it doesn't start compiling again. I guess it doesn't matter because it's so old. IMO this is one of these "don't waste time thinking about it, just do it" kind of things.
There was a problem hiding this comment.
Ok, though I might trim some of these back again another time if I ever try to optimise our CI pipeline. 👍
| @@ -722,7 +722,7 @@ If `T` implements `Clone`, you can extract `T` itself. | |||
| In addition, you can also extract `&PyCell<T>`, though you rarely need it. | |||
|
|
|||
| Before: | |||
| ```ignore | |||
| ```compile_fail | |||
There was a problem hiding this comment.
Ok, though I might trim some of these back again another time if I ever try to optimise our CI pipeline. 👍
| @@ -311,7 +311,7 @@ from anywhere as long as your `app.py` is in the expected directory (in this exa | |||
| that directory is `/usr/share/python_app`). | |||
|
|
|||
| `src/main.rs`: | |||
| ```ignore | |||
| ```no_run | |||
There was a problem hiding this comment.
I was just wondering whether the rust tag was also needed to give nice syntax highlighting.
