Redesign Python relationship into wrapper

[henryiii]

This could be a very large change, so I'd like to see what other think about it. This is the possible idea, proposed by Hans some time ago:

The "histogram" class would become a pure python class, with a _hist private member that holds the C++ histogram object. All calls would basically forward to the underlying _hist methods. For simplicity below, I'll call the Python histogram class "Histogram" to separate it from histogram - it does not need to be called this in the design. Design highlights:

• Caching axes is trivially done in __init__
• The cpp histogram class can be very close to the cpp design (and should be, to reduce cognitive load)
• The Histogram class can have a parent BaseHistogram class (ignore my naming for now), that does not support addition and such but still has axes, that can be used as the output of .density()
• Either the cpp histograms or a new subclass can be used to provide a highly Boost.Histogram C++ compatible syntax in boost_histogram.cpp. Without this, we will probably need to inject and remove methods to provide a compat mode.
• The current trickery designed to make the multiple histogram classes isinstance as one goes away. Same thing for axis.* (which also get this treatment). We don't have to do the method injection loops either.
• Docstrings can be written in Python.

Downsides:

• All calls wrapped - tight loops might be a bit slower.
• Adds another level of indirection, another level of design complexity - might be especially irritating when building a new library on top of this as a back end, like hist/physt/etc. Makes me think of Cython...
• All calls creating a histogram (like slice indexing) would need to also wrap the output to turn it back into a self.__class__.
• Things like buffer view have to be setup again in Histogram.
• Signatures need to be crafted in Python, since all calls are redirected. But, we might be doing this anyway for nicer Python 3 signatures. Python 2 might get sub-standard *args, **kwargs signatures everywhere, but I'm not that interested in making Python 2 interactive work beautiful.

Example:

class BaseHistogram(object):
    def __init__(self, ...):
        self._hist = make_histogram(...)
        self.axes = AxesTuple(... self._hist ...)

    def __getitem__ ...
class Histogram(BaseHistogram):
    def __add__ ...

What do you think? Is the extra flexibility and simpler Python structures worth the extra complexity? This would help with #174 and #155.

[henryiii]

If this is the way we go, in Hist we could potentially just subclass Histogram, then add plotting, etc. It creates a tight connection there, but reduces the duplicate work. Aghast would then be used as the connector between Hist/boost-histogram and the outside world, instead of also the connector between Hist and boost-histogram.

[HDembinski]

This was my original idea and proposal as you remember, so obviously I am in favor despite the downsides, which I consider not very harmful. I don't think the performance issues are really issues and it allows us to write (some) pythonic enhancements directly in Python. Building classes from inheritance is a completely normal design approach in Python. I wouldn't call this indirection.

[HDembinski]

Besides, I already demonstrated that calling a method on a derived class is as fast as calling it on the base.

[henryiii]

Classes from bases is not the performance issue here, it's the h.xxx() calls self._hist.xxx() that does incur a penalty and is indirection. We can't inherit from the original C++ classes because there are many of them, not one. After the initial BaseHistogram thing, the rest is normal Python inheritance.

But I also agree that the downsides are not very harmful - at least 1 indirected call shoudn't kill us - anyone directly looping in Python is paying a penalty anyway. I'm more worried about the extra complexity, but it buys us some simplicity too. The design has more Python in it than it did before, so I'm now inclined in this direction, too. (Yes, it was your suggestion a while ago).

[henryiii]

It also has the + that all doc strings can be written in Python. :)

[henryiii]

@HDembinski, unless you think of something else and change your mind, or @jpivarski has any ideas, we will probably go this way soon(ish). (As always, anyone else is free to provide input, too - this is mostly internal, though, users will not see much of a change at all at first).

[HDembinski]

It also has the + that all doc strings can be written in Python. :)

Which was also one the advantages that I pointed out, as you may remember.

[jpivarski]

I think putting a Python layer between the user and the implementation is fine. In fact, I'd consider it wise, even if no specific applications are foreseen (much like hiding attributes behind properties) because adding new features for Python-friendliness would now have a very low bar (no recompilation, no need to rexpress in pybind11 idioms, which, nice as they are, are still less convenient than Python itself).

I don't think there should be any concern about the performance of calling Histogram methods. Herculean efforts to streamline tight loops written in Python can at best gain a factor of few over the simplest implementation. Users should put their tight loops on the C++ side (i.e. h.fill(array) instead of for x in array: h.fill(x)) or suffer the consequences. Dividing the world into "fast math" and "slow control" is not only more effective in the long run, but it's liberating when writing the "slow control" code to be able to completely ignore questions of performance.

[henryiii]

Looks like we have a consensus, then. :)

I wasn't that worried about performance, I was more worried about the extra design verbosity; but using "normal" python idioms instead of the current injection and trickery is probably easier in the long term; it's familiar, though more wordy and repetitive. And reduces recompiles.

[HDembinski]

Either the cpp histograms or a new subclass can be used to provide a "compat" mode in boost_histogram.cpp. Without this, we will probably need to inject and remove methods to provide a compat mode.

Why is there a need for a compat mode? I thought the cpp histogram is the "compat" mode. I don't want it to be called that, because compat sounds like this a legacy.

[henryiii]

The stuff in boost_histogram.cpp would be highly compatible with Boost.Histogram's syntax, that's all I intended to say for the wrapped version. It's not really a mode. If we didn't provide the wrapper, we'd probably have to inject/remove methods, which would be a mode of sorts.

[HDembinski]

I wasn't that worried about performance, I was more worried about the extra design verbosity; but using "normal" python idioms instead of the current injection and trickery is probably easier in the long term; it's familiar, though more wordy and repetitive. And reduces recompiles.

I remember that you used performance as a counterargument when I proposed this, so it is interesting that you now use my arguments as your own. I am glad that you now agree that this is the best course of action, but everyone who reads this should know that I pushed for this design from the very beginning against your resistance. All the arguments presented here I already gave when we discussed this.

[henryiii]

The idea was yours originally, but the design was quite different then, we didn't have a major pure-python part. You probably had a better idea of the big picture then and where we were going. I was focused on the next step.

And there are some downsides. The current design allows a single method definition, while we will have to reimplement every single method that we want to expose in Python as well now. Wanting to provide different "flavors" of Histograms (both for density histograms and cpp histograms) is what pushed me over the edge.