Ideas and progress

[fafhrd91]

1. Seems rust-cpython is on hold right now, no changes, pr hangs without attention for months.
2. as I have now enough experience with rust_cpython py_class implementation, I think it is not good solution. It is separate language within rust. it is very hard to understand and extend.

I work on new py class implementation based on procedural macros and specialization, but that requires rust nightly. should be ready in several days.

[fafhrd91]

new code will look something like this

#[py_class]
struct TestClass {
    data: Cell<bool>
}

#[py_impl]
impl TestClass {

  #[method]
   fn test_method(&self, val: PyObject) {}
}

#[py_impl]
impl PyObjectProtocol for TestClass {

    fn __call__(&self, args: PyTuple, kwargs: PyDict) -> PyResult<> {
    }

   ...
}

#[py_impl]
impl PyAsyncProtocol for TestClass {
   fn __await__(&self) -> PyResult<PyObject> {}
}

[fafhrd91]

@1st1 @asvetlov would you like guys to participate in design work?

[fafhrd91]

py_class may accept parameters,

#[py_class(MyFuture, base=asyncio::Future)]
struct PyFut {}

[fafhrd91]

big question at the moment, how to parse arguments, especially *,

[ohanar]

I agree that rust-cpython's class design isn't ideal. I would prefer to be able to not tie this library to the nightly compiler, if instead this functionality could be done through a custom derive. Something along the lines of the following:

#[derive(PythonClass)]
#[py_methods(my_method(u64, String), my_other_method)]
#[py_traits(PyFunction(PyTuple, ())]
struct MyClass(...);

impl MyClass {
    fn my_method(&self, x: u64) -> PyResult<String> {
        ...
    }
    fn my_other_method(&self) {
        ...
    }
}

impl PyFunction for MyClass {
    fn __call__(&self, y: PyTuple) -> PyResult<()> {
        ...
    }
}

It obviously isn't as nice as what you get with custom attributes (and maybe you could do better than this with some hackery, haven't really thought about it too much), but it should work with stable rust.

[fafhrd91]

problem is how to define special methods, like PyAsyncMethods of PyNumberMethods. you need access to impl block, and you need to enumerate and generate description during macro expansion.

I couldn't find any other way :)

[fafhrd91]

right now, special method implementation is very simple, for example sequence protocol https://github.com/PyO3/PyO3/blob/master/src/class/sequence.rs#L19
btw sequence protocol is not implemented by rust-cpython properly, because of ambiguity of method definitions

[fafhrd91]

added methods and properties implementation

#[class]
struct MyClass {}

#[methods]
impl MyClass {

  fn method(&self, py: Python, arg: PyObject) -> PyResult<()> {
     Ok(())
  }

  #[getter]
  fn get_prop1(&self, py: Python) -> PyResult<PyObject> {...}

  #[setter]
  fn set_prop1(&self, py: Python, value: PyObject) -> PyResult<()> {...}

  #[getter(prop2)]
  fn get_some_prop(&self, py: Python) -> PyResult<PyObject> {...}

  #[setter(prop2)]
  fn set_some_prop(&self, py: Python, value: PyObject) -> PyResult<()> {...}

}

next is "#[class]" macro and argument parsing

[ohanar]

One thing that I like about rust-cpython is that I don't have to manually convert each argument and return result (although I can if I want to). Also, I don't like that there are currently dangling methods when you implement protocols. What about something like this:

trait PySequenceProtocol {
    fn __len__(&self, py: Python) ->Self::Result where Self: PySequenceLenProtocol;
    fn __getitem__(&self, py: Python, key: Self::Key) -> Self::Result where Self: PySequenceGetItemProtocol;
    ...
}

trait PySequenceLenProtocol: PySequenceProtocol {
    type Result: IntoPyResult<Target=usize>; 
}

trait PySequenceGetItemProtocol: PySequenceProtocol {
    type Key: From<isize>; // or maybe something like TryFrom<isize, Err=Self::Result>
    type Result: IntoPyResult;
}

I'll try to prototype something along these lines latter tonight.

[fafhrd91]

arg parser is next after #[class]

regarding dangling methods, those are visible only in rust code, python code does not see unimplemented methods. I don't know how to make method signature explicit and optional at the same time

[fafhrd91]

py_class! is dropped, #[class] is implemented.

[ohanar]

Unfortunately I didn't have as much time as I would like tonight to work on this. I pushed what I did tonight here: 49c3502. The idea is to make rust's trait system do most of the heavy lifting. I haven't touched the attribute macros yet, but all that it should need to do for slotted methods is take something like the following:

#[py_impl]
impl PyMappingProtocol for MyClass {
    fn __len__(&self, py: Python) -> usize {
        ...
    }

    fn __getitem__(&self, py: Python, key: String) -> PyResult<i32> {
        ...
    }
}

And add the following marker trait impls:

impl PyMappingLenProtocol for MyClass {
    type Success = usize;
    type Result = usize;
}

impl PyMappingGetItemProtocol for MyClass {
    type Key = String;
    type Success = i32;
    type Result = PyResult<i32>;
}

After that is done, then some corresponding Impl traits will wrap the methods appropriately, and bundle them up into a PyMappingMethods struct (with null pointers in any slot that hasn't had it's marker trait impl'ed).

I'll try to work more on this over the next couple of days.

[fafhrd91]

That's awesome. I'll play with this idea too

[fafhrd91]

@ohanar
it is not possible partially implement protocol, for example if I need inly __getitem__ method, nothing else, with this approach, I have to implement all methods. imaging something like PyNumberProtocol.
but I really like you idea! maybe we can somehow adjust implementation in attribute macro?

[fafhrd91]

we can add default method implementation with attribute macro

[ohanar]

Yes, I noticed that right after writing my post, and added a default implementation: 3be2c9c. Because of the marker traits, those methods don't really exist unless you implement the corresponding marker trait (the rust compiler just won't let you call them), so the method is more or less unchanged.

[fafhrd91]

that is so awesome! I love rust :)

[fafhrd91]

@ohanar I implemented mapping interface with your idea! 2eea45e

[ohanar]

Very cool! I'm glad the macro code is as straightforward I hoped (admittedly I haven't yet written any macros using syn+quote, but I've read through their docs, and it seemed like it would be straightforward).

Now we just need macros to generate all of the traits and their impl's :).

[fafhrd91]

yes, that will take some time

[ohanar]

FYI, I'm working on a macro to help with this.

[fafhrd91]

@ohanar very good!

could you check this code https://github.com/PyO3/PyO3/blob/master/src/conversion.rs#L136
we don't really need to implement ToPyObject for new classes,
impl<T> ToPyObject for T where T: PythonObject should provide default implementation. but I can not make it work because we have another
impl <'a, T: ?Sized> ToPyObject for &'a T where T: ToPyObject
I am not sure how to resolve conflict or if it event possible.

[ohanar]

This can't currently be resolved. The issue is rust doesn't know which impl to use for an &T where T: ToPyObject, &T: PythonObject. Obviously we know that these shouldn't exist, but there is no way to encode that in rust. Unfortunately, specialization (currently) doesn't help us either, since it is restricted to specializing subsets (and neither one of these is a subset of the other).

I personally think that the impl for T: PythonObject is more useful, since if you have a ref &T and you call to_py_object on it, rust should automatically deref it and see the implementation for the underlying T. (This doesn't work with into_py_object since that consumes T and derefing only allows you to use &self or potentially &mut self methods.)

[ohanar]

I pushed my work in progress of the boilerplate generator to my fork. I probably won't have time to work on it much more for a couple of days.

It needs a little more work on the parsing side of things (mainly needs to handle special edge cases, like __setitem__/__getitem__, and quite a bit more work on the code generation side. Right now it just generates the public facing traits, so long as all of the arguments and the return type are python objects.

[fafhrd91]

I came to similar conclusion on ToPyObject. In any case trait get generated by macros, it could wait

[fafhrd91]

I added ability to specify py methods in protocol, some protocol fn needs to be py methods and not slots, i.e. __aenter__ but open opportunity to extent protocols beyond just slots. We can add keys, items etc to PyMappingProtocol

[wdv4758h]

This fork is interesting !

I'm new to rust-cpython, and still trying to figure out what things has done by the macro. In my trying, I start to think it should be a better way, and you guys look like already working on it !!! 👍 😃

[fafhrd91]

@ohanar i am not sure how to use reference in assitiated type without attaching lifetime everywhere

[fafhrd91]

ok, figured this out myself.

[fafhrd91]

it seems it is not possible to use associated types for protocol implementation if type needs to be generic over lifetime (references), we need HKT for this.

am I wrong?

[ohanar]

I haven't had the chance to look at your pyptr branch in depth enough to see why things are failing, but I'm not sure that we need HKTs for this. I think my original signature was a slightly off; I think it should be something more like this:

trait PyMappingProtocol: ... {
    fn __setitem__<'a, 'b>(&self, /* py: Python */, key: Self::Key, value: Self::Value) -> Self::Result
        where Self: PyMappingSetItemProtocol<'a, 'b>
    { unimplemented!() }

   ....
}

trait PyMappingSetItemProtocol<'a, 'b>: PyMappingProtocol {
    type Key: FromPyObject<'a>;
    type Value: FromPyObject<'b>;
    type Success: ToPyObject;
    type Result: Into<PyResult<Self::Success>>;
}

That is methods should be generic over a lifetime for each python object argument. If you still run into issues with this, we could always remove the lifetime parameter from FromPyObject and make it dual to ToPyObject in the sense that it always will clone data. (I think the only place where an actual borrow happens at the movement is if you try to convert a python utf8 string into a rust &str or Cow<str>. The only other place that I think you might be able to do that is converting a python bytearray into a rust &[u8] or Cow<[u8]>.)

[fafhrd91]

I already removed <'a> from FromPyObject. And if you need access to bytearray, it is possible just to downcast into PyBytesArray and use data directly.

[fafhrd91]

experimenting with multiple lifetimes

[ohanar]

It's important the lifetimes be on the methods and method traits, not on the protocol traits.

[fafhrd91]

I am closing this ticket. all ideas get merged into master