Use PyO3 functionality to remove Python files

[Radonirinaunimi]

Towards addressing #200.

TODO:

• improve and/or add documentations $\leftarrow$ done through Update Python docs #301
• rename instances of *lumi* into *channel* to be consistent with the pineappl crate $\leftarrow$ done through Update Python docs #301

[cschwan]

Thank you for taking care of this, @Radonirinaunimi! Whenever you need help, let me know!

[felixhekhorn]

Thank you for taking care of this, @Radonirinaunimi! Whenever you need help, let me know!

same here!

[Radonirinaunimi]

Although:

• renaming instances of *lumi* into *channel* to be consistent with main crate (in crates, functions, variables, etc.) done through Update Python docs #301
• fixing and/or adding some documentations done through Update Python docs #301
• doing more tests to make sure that all the corner cases did not break without us knowing
• improving handling of exceptions and errors currently not strictly required as (all) python PyErr have From conversion to match rust Err.

are still to be done, this is mostly ready (in that it passes all the tests) so I think you can already have a look.

There are only a few things that are not exactly the same as before (and that we may or may not want to address):

• the methods .create() and .from_grid() are no longer available, instead objects are instantiated from the class
• .into() needs to be manually called on subgrids in order to convert into SubgridEnum

PS: I could not force-push so please ignore the commit history 😇

[cschwan]

There are only a few things that are not exactly the same as before (and that we may or may not want to address):

* the methods `.create()` and `.from_grid()` are no longer available, instead objects are instantiated from the class

* `.into()` needs to be manually called on subgrids in order to convert into `SubgridEnum`

That's perfectly fine, and quite happy that we can get away with so few changes actually.

PS: I could not force-push so please ignore the commit history 😇

I've disabled force-push for the time being since I'm not convinced about it being a good idea.

[felixhekhorn]

• the methods .create() and .from_grid() are no longer available, instead objects are instantiated from the class

I think this is acceptable - but makes this a breaking change.

• .into() needs to be manually called on subgrids in order to convert into SubgridEnum

this is quite "rusty" and not "pythonic", if we can't find a way around we can keep it to honour our underlying language - but if possible I'd like to avoid. Can we do something with the Into trait? i.e. implement that for the subgrids (i.e. move the function to an impl block) and then take the trait as argument of PyGrid::set_subgrid? (No idea if PyO3 will like it)

[Radonirinaunimi]

this is quite "rusty" and not "pythonic", if we can't find a way around we can keep it to honour our underlying language - but if possible I'd like to avoid. Can we do something with the Into trait? i.e. implement that for the subgrids (i.e. move the function to an impl block) and then take the trait as argument of PyGrid::set_subgrid? (No idea if PyO3 will like it)

I've had a look at this a bit and not only it could become really complicated but also messy. What would be the reasons we should avoid the current approach (apart from breaking changes)? It is basically a difference between doing:

set_subgrid(order, bin, lumi, subgrid.into())

vs before when it was re-wrapped inside:

pineappl/pineappl_py/pineappl/grid.py

Lines 127 to 143 in 8c12335

	def set_subgrid(self, order, bin_, lumi, subgrid):
	"""Set the subgrid at the given position.
	Convenience wrapper for :meth:`pineappl.pineappl.PyGrid.set_subgrid()`.
	Parameters
	----------
	order : int
	index of order
	bin_ : int
	index of bin
	lumi : int
	index of luminosity
	subgrid : ImportOnlySubgridV1
	subgrid content
	"""
	self.raw.set_subgrid(order, bin_, lumi, subgrid.into())

[cschwan]

set_subgrid(order, bin, lumi, subgrid.into())

Where do we use Grid::set_subgrid? This will need to changed, since soon (#299) all the subgrid types will be changed/removed.

[felixhekhorn]

Where do we use Grid::set_subgrid? This will need to changed, since soon (#299) all the subgrid types will be changed/removed.

yadism does - since we do the interpolation ourselves

What would be the reasons we should avoid the current approach

as said above the only reason is being "rusty" and not "pythonic" - if it is too much of a hassle it is also fine ... (as Python knows no (strong) types, it also doesn't know casting so it feels a bit weird, that's it)

[Radonirinaunimi]

as said above the only reason is being "rusty" and not "pythonic" - if it is too much of a hassle it is also fine ... (as Python knows no (strong) types, it also doesn't know casting so it feels a bit weird, that's it)

Ok, let's keep it as it is then.

I think this is basically done. The remaining todos re docs and change of macros were already addressed by #301. The only maybe we'd add is a pre-commit hooks to format the python tests (and that also runs cargo fmt)(?).

[Radonirinaunimi]

Are we happy to merge this now? We can add pre-commit hooks to automatically run black and cargo fmt before commit in another PR.

[cschwan]

Are we happy to merge this now?

I found some minor things, let me fix them first.

[Radonirinaunimi]

@cschwan What was actually the reason of renaming/moving channel into boc?

[cschwan]

@cschwan What was actually the reason of renaming/moving channel into boc?

That just reflects the same module structure in Rust. I think we should keep the two as close as possible.

[Radonirinaunimi]

That just reflects the same module structure in Rust. I think we should keep the two as close as possible.

Yes, this I understood and I completely agree. But what was the reason to change it in the main Rust module (for my own curiosity).

[cschwan]

But what was the reason to change it in the main Rust module (for my own curiosity).

Grid is basically, as you know, a three-dimensional array over bins, orders and channels, and I wanted to have a single module to reflect these three dimensions. It's not perfect yet, as there's still the bin module and the ordering isn't boc but obc, but I'd expect that to change in the future.

[Radonirinaunimi]

Grid is basically, as you know, a three-dimensional array over bins, orders and channels, and I wanted to have a single module to reflect these three dimensions. It's not perfect yet, as there's still the bin module and the ordering isn't boc but obc, but I'd expect that to change in the future.

I see, thanks!

[cschwan]

@Radonirinaunimi can you please check that commit 8ef2297 doesn't do anything unexpected? Contrary to what I wrote it isn't a bug, in the new commit it should just be more succinct. If it's OK, you can merge!

[Radonirinaunimi]

@Radonirinaunimi can you please check that commit 8ef2297 doesn't do anything unexpected? Contrary to what I wrote it isn't a bug, in the new commit it should just be more succinct. If it's OK, you can merge!

I checked and this does not change anything, so we are good.