NNPDF / pineappl

PineAPPL is not an extension of APPLgrid

https://nnpdf.github.io/pineappl/

GNU General Public License v3.0

13 stars 3 forks source link

Update Python docs #301

Closed felixhekhorn closed 3 months ago

felixhekhorn commented 4 months ago

Closes #157

This mostly updates and fixes all Python doc strings.

Apart from that there are few more (even breaking) changes:

PyGrid::evolve got deprecated (as the underlying function)
PyFkTable::convolve_with_two: return type was harmonized with PyFkTable::convolve_with_one to avoid the deprecation of into_pyarray
the transition "Lumi" -> "Channel" was performed both on the PyO3 and Python side

if necessary we could also add other changes to the Python interface here, such as to expose Grid::convolutions

cschwan commented 3 months ago

157 is a related Issue.

felixhekhorn commented 3 months ago

Are you also planning to modify the occurrences of lumi(_) in the argument of the functions and in the variables names of src/fk_table.rs, src/grid.rs, and pineappl/grid.py?

I hope I caught all remaining occurrences - the only remaining appearance of the word "lumi" in pineappl_py/src is related to pineappl::convolutions::LumiCache, which I did not touch (as the underlying Rust is not touching it).

There is some overlap with #302 since both are touching the files in pineappl_py/src

cschwan commented 3 months ago

I ran cargo fmt in commit fd6f4226ecaf5dd57b253f2653c32844ca5e4b06. @felixhekhorn, could you check that these changes don't break anything?

cschwan commented 3 months ago

I can merge this now if you like, for wrappers of Grid::{convolutions,set_convolutions} we should open a new PR.

When this will be merged into master, I'm going to release this as a new minor version (0.9.0) instead of increasing the patch version of 0.8, because we renamed publicly-visible items.

felixhekhorn commented 3 months ago

I ran cargo fmt in commit fd6f422. @felixhekhorn, could you check that these changes don't break anything?

looks good

we renamed publicly-visible items.

indeed, I know that the channel_mask parameter for evolve will break some of my scripts ...

cschwan commented 3 months ago

We need a strategy for merging this, since #302 removes all Python files and thus the changed documentation. So either I

merge this PR first, then #302, which means that in #302 the updated documentation has to be put in front of the Rust structs (probably),
or I merge #302 first, which means the changes from this PR will have to be rewritten here.

@felixhekhorn @Radonirinaunimi what path do you believe is best?

felixhekhorn commented 3 months ago

merge this PR first, then

I'm in favour of option 1:

I'm resolving here the TODO for lumi/channel
there are no change to docs in #302
this will trash my changes here to pineappl_py/pineappl, but, fine, happens, we gain something else

if we can resolve #302 quickly they can go into one version (as both are breaking)

Radonirinaunimi commented 3 months ago

If this is ready it could be merged first and then I will resolve the conflicts in #302. The advantage of this is that this will address some of the todos in #302. If instead #302 is ready first then we reverse the order.