Move python bindings

[alecandido]

Move python bindings into main pineappl crate

[alecandido]

First question: the obvious thing to do it's to port all the public methods of the required structs to python (and at the end of all the public structs), but of course some of them are not just working out of the box.

Should I do like for the objects, that I'm just implementing some of them for the time being, and so split the impl blocks in those that are going to collect the immediately pythonized objects and those that are waiting, or instead should I try to port all of them immediately?

[cschwan]

Before I answer your question, please use the cfg and cfg_attr attributes as shown in the last commit. Is it possible that I can't compile the entire crate yet?

[alecandido]

I'm trying, but unfortunately the answer was not already before. However I can tell you that now we have brand new problems:

1. not a problem, just a remark, now you have to run like this:

maturin develop --cargo-extra-args="--features python"

2. the #[new] macro looks like it's not working in the new style:

error: cannot find attribute `new` in this scope=====================> ] 81/82: pineappl
   --> pineappl/src/bin.rs:164:36
    |
164 |     #[cfg_attr(feature = "python", new)]
    |                                    ^^^

and same for #[pyclass] and #[pymethods]:

error: cannot find attribute `pyclass` in this scope=================> ] 81/82: pineappl
   --> pineappl/src/grid.rs:178:32
    |
178 | #[cfg_attr(feature = "python", pyclass)]
    |                                ^^^^^^^


error: cannot find attribute `pymethods` in this scope===============> ] 81/82: pineappl
   --> pineappl/src/grid.rs:189:32
    |
189 | #[cfg_attr(feature = "python", pymethods)]
    |                                ^^^^^^^^^

Are you sure that #[cfg_attr(...)] is suitable for macros?

[cschwan]

As far as I understand it should work, see https://doc.rust-lang.org/reference/conditional-compilation.html#the-cfg_attr-attribute. For time being you can simply revert all changes.

[cschwan]

It seems that cfg_attr and new don't play well together: PyO3/pyo3#780

[alecandido]

I would like to keep them, since I really like more the bindings organized this way than just spread all over, without any way to strip them off.

[cschwan]

Concerning your original question: I'd suggest that first tackle the most important methods and objects, just as in pineappl_py. We can then import more as soon as we need them.

Which methods don't work out of the box?

[alecandido]

It seems that cfg_attr and new don't play well together: PyO3/pyo3#780

I'm looking in the thread for hints, seems like not always is a pyo3 fault, even if I still don't fully understand:

@davidhewitt There are no errors with pymethods or pyclass because I forgot to enable the macros feature flag. I am sorry for the fuzz, should have paid more attention to the changelog.

Concerning the former question:

Concerning your original question: I'd suggest that first tackle the most important methods and objects, just as in pineappl_py. We can then import more as soon as we need them.

Fine, but actually I raised the question too early

Which methods don't work out of the box?

Indeed, they few methods in impl BinRemapper are actually all extremely simple, but the problems are with the types:

   --> pineappl/src/bin.rs:166:25
    |
166 |         normalizations: Vec<f64>,
    |                         ^^^ the trait `From<&PyCell<BinRemapper>>` is not implemented for `Vec<f64>`
    |

   ::: /home/alessandro/.cargo/registry/src/github.com-1ecc6299db9ec823/pyo3-0.12.4/src/callback.rs:170:8
    |
170 |     T: IntoPyCallbackOutput<U>,
    |        ----------------------- required by this bound in `pyo3::callback::convert`
    |

and so on, so it's not playing well with the following types:

• Vec<f64>
• Vec<(f64,f64)>
• &[f64]
• &[(f64,f64)]

that actually were perfectly fine (at least the first two) inside pineappl_py. So I'm looking for the solution...

[cschwan]

@alecandido : that's strange, I'll have a look at it.

[alecandido]

Actually the referred option is described here. I wonder if we should add it explicitly: before it was working fine (and it is on the old branch), so I imagine that either pyo3 or maturin are already providing it somehow.

[cschwan]

I'm closing this since we can't use #[cfg_attr(feature = "python", pymethods)]. The current approach in #51 is the right one.

[alecandido]

Maybe there will be some chance later on: PyO3/pyo3#780 (comment)