emit deprecation on pyclasses that are affected by the FromPyObject blanket impl#5550
Conversation
36cd3f7 to
762ef5f
Compare
davidhewitt
left a comment
There was a problem hiding this comment.
Thanks, generally looks great to me, nice touch to only emit it on types which are actually Clone!
I think also worth noting in the migration guide?
| | | ||
| = note: available fields are: `0`, `1`, `2` | ||
|
|
||
| warning: use of deprecated associated constant `pyo3::impl_::deprecated::DeprecatedFromPyObjectBlanket::<true>::MSG`: Implicit by value extraction of pyclasses is deprecated. Use `from_py_object` to keep the current behaviour or `skip_from_py_object` to opt-out. |
There was a problem hiding this comment.
I feel like it would be helpful here to make the hint a bit clearer that this is applicable to classes which implement `Clone.
Maybe something like the following (note I renamed the struct too given it shows in the public error message):
| warning: use of deprecated associated constant `pyo3::impl_::deprecated::DeprecatedFromPyObjectBlanket::<true>::MSG`: Implicit by value extraction of pyclasses is deprecated. Use `from_py_object` to keep the current behaviour or `skip_from_py_object` to opt-out. | |
| warning: use of deprecated associated constant `pyo3::impl_::deprecated::HasAutomaticFromPyObject::<true>::MSG`: The automatically derived `FromPyObject` implementation for `#[pyclass]` types which implement `Clone` is being phased out. Use `from_py_object` to keep the automatic derive or `skip_from_py_object` to accept the new behaviour. |
762ef5f to
3e0c463
Compare
|
Just asking this in user context. Is it the case that when the
Otherwise the deprecation warnings seem very clear right now, as a newcomer to this topic just upgrading to the latest release. But can become a bit clearer if the docs do make it a bit crisper about what you lose in user code when the Or just actually, commenting on how are Cloning and converting from python coupled. I'll be reading https://github.com/PyO3/pyo3/blob/main/guide/src/conversions/traits.md in parallel. |
Yes!
No, it's the other direction. To implement |
This emits a deprecation warning on
#[pyclass]es that are affected by theFromPyObject + Cloneblanket impl. It uses aProbeto detect theCloneimpl and conditionally emits the deprecation. This ensures only types are flagged that are actually affected by the change. The deprecation can be suppressed by specifying either theskip_from_py_objectorfrom_py_objectoption.With this in 0.28, I think we can remove the blanket impl in 0.29.
#5419