deprecate super class initialization from tuples

[Icxolu]

This deprecates the tuple syntax for super class initialization as per #5739 (review).

In #[new] this will emit a deprecation warning when detected. As for the From<(S, B)> for PyClassInitializer<S> impl, I'm not sure we can do more that mentioning it in the changelog and migration guide.

[davidhewitt]

Thanks, this looks good to me. Some thoughts:

• We should probably touch up the docs and guide to make it easier for users to understand how to use PyClassInitializer.
• Should we be figuring out how to pass arguments to super __new__ before asking users to migrate existing code? I am not entirely sure we won't have to change the PyClassInitializer API to make that possible, and it might be unfortunate if we push users to use this API just for us to change it again.

[davidhewitt]

This doc needs updating, probably should just mention PyClassInitializer here.

[davidhewitt]

Should remove this too.

[Icxolu]

• Should we be figuring out how to pass arguments to super __new__ before asking users to migrate existing code? I am not entirely sure we won't have to change the PyClassInitializer API to make that possible, and it might be unfortunate if we push users to use this API just for us to change it again.

Good question. I think this depends on how we want that API to look like. The simple solution would probably be to add something like PyClassInitializer::add_native_base_args and store them inside of the PyClassInitializer. I think this could work to provide that functionality, but I'm not sure if that's the best api we can come up with...

For example something like this:

impl<T: PyClass> PyClassInitializer<T> {
    ...

    pub fn add_args(mut self, args: Bound<'_, PyTuple>) -> Self
    where
        T::BaseType: PyClassBaseType<Initializer = PyNativeTypeInitializer<T::BaseType>>,
    {
        self.args = Some(args.unbind());
        self
    }
    
    pub fn add_kwargs(mut self, kwargs: Bound<'_, PyDict>) -> Self
    where
        T::BaseType: PyClassBaseType<Initializer = PyNativeTypeInitializer<T::BaseType>>,
    {
        self.kwargs = Some(kwargs.unbind());
        self
    }

    ...
}

And usage like this:

#[pyclass(subclass, extends=PyDict)]
struct Foo {}

#[pyclass(extends=Foo)]
struct Bar;

#[pymethods]
impl Bar {
    #[new]
    fn new(py: Python<'_>) -> PyClassInitializer<Self> {
        let args = todo!();
        PyClassInitializer::from(Foo {}).add_args(args).add_subclass(Bar)
    }
}

[davidhewitt]

I think that API could certainly work, I also have a few scattered thoughts which might influence us:

• I realise that PyClassInitializer<T> is slightly unfortunate in that it moves the T twice; once into the PyClassInitializer and then again into the final class object. I wonder if we could rework PyClassInitializer internally so that it is just a thin wrapper around the *mut PyObject partially-initialized return value from super __new__, and add_subclass etc just write to the class object.
  • There are weird edge cases like what if super __new__ returns an existing object? I think we already probably don't handle that case correctly, I wonder if we can even demonstrate crashes and/or unsoundness from that already?
  • Arguably if super __new__ returns an existing object, there should be no need to produce any new T for the current type at all, it'd be nice if we could write an API which supports that pattern.
• I wonder if we can make the experience for users feel much like Python, e.g. (probably not possible but to throw out ideas)

  // super().__new__(cls, args, kwargs)
  let new_obj: PySuperInitializer<T> = pyo3::super_new(cls, (arg1, arg2), None)?; 
  // check if new_obj is existing somehow?
  // write Rust state
  let new_obj: PyClassInitializer<T> = new_obj.add_class(Self { x, y, z });
  // safely cast to `Py<T>
  new_obj.finish()

[davidhewitt]

I believe #4443 is touching on the same issue (albeit just for native supertypes).