Skip to content

Conversation

@henryiii
Copy link
Member

@henryiii henryiii commented Oct 31, 2019

This changes the design to hold an internal object, and users only interact with the Python classes. Closes #180. Proposed by @HDembinski some time ago.

File layout changes

The C++ core is now named _core, and non-public Python parts are in _internal. The cpp module is new (see below). AxesTuple is in now in _internal/axestuple.py, _utils is now _internal/kwargs.py (the old meta utils are gone, no longer needed). uhi.py is now tag.py.

Histogram design:

  • BaseHistogram: Shared interface
    • BoostHistogram: Mimics the C++ interface as closely as possible. Even has __call__!
    • Histogram: The default interface, with Pythonizations. Caches the axes tuple.

You can convert between the two, or add a new class. If you do:

import boost_histogram.cpp as bh

You will now be working with BoostHistograms instead of Histograms. (closes #155)

Axes design:

There is a base class Axis, along with Regular, etc. for the 4 types. The axis object is stored in _ax. The function forms are now class methods, regular.sqrt for example (they are still available as regular_sqrt for the moment, as well). A helper function internally knows how to convert an axis object into the correct Axis class, and is used when pulling an axis from a histogram.

General changes

The underscores on some C++ parts have been removed. An underscore in the C++ bindings indicates that something is different than the C++ code. The C++ _core objects have been stripped down a little, and exactly mimic C++ where possible (.rank() is a function, etc). The Pythonisation now happens in Histogram. Axes tuples have functions now too (closes #176), and are finally tested.

Also, more docs present, closes #174.

@henryiii
Copy link
Member Author

henryiii commented Nov 1, 2019

Let's go ahead and merge this into develop, and review/changes can be PRs. I need this in so I work on the next set of changes as new branches on the flight to Australia. Anything here can be reverted/modified in develop if needed.

@henryiii
Copy link
Member Author

henryiii commented Nov 1, 2019

(Actually, @HDembinski, to make it easier to review, I'll leave this open, but build branches off of it on the plane, but will need to merge before proceeding).

@henryiii henryiii merged commit 9efe55c into develop Nov 2, 2019
@henryiii henryiii deleted the henryiii-wrapper branch November 2, 2019 02:55
@henryiii
Copy link
Member Author

henryiii commented Nov 2, 2019

Please raise issues if there are parts of the design that could be better! I've already been working off this, and it seems to be working pretty well.


#### User changes

* You can now access the functional regular axis directly, `regular_sqrt` becomes `regular.sqrt`, etc.. [#183][]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No regular.sqrt. The transform should be a passed as a keyword to the regular class.

else:
raise KeyError("Unsupported collection of options")

@classmethod
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a change that slipped by me. I think we should not have these class methods, since people should use the transform keyword.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants