feat: __dict__ major simplification #477
Conversation
|
@HDembinski I need to implement this on all axes types, but before I do, does this look reasonable? It makes the "this is a normal |
|
Looks ok to me. |
henryiii
force-pushed
the
feat/__dict__
branch
from
7ebc88b to
7398693
Compare
henryiii
force-pushed
the
feat/__dict__
branch
from
eae70c9 to
2269df8
Compare
henryiii
force-pushed
the
feat/__dict__
branch
from
2269df8 to
e7e96d2
Compare
|
Major simplification in last commit (please look at it by itself, you can't get a full sense for how much this simplified without a direct comparison with matching features!) I came across a mention in the Python docs that you can have a mixed This fixed a bug with |
henryiii
force-pushed
the
feat/__dict__
branch
from
e7e96d2 to
80aa9f2
Compare
|
|
||
| def __getattr__(self, item): | ||
| if item == "_ax": | ||
| return Axis.__dict__[item].__get__(self) |
There was a problem hiding this comment.
This sort of thing is no longer needed! :)
| @@ -553,7 +592,9 @@ def _repr_args(self): | |||
| @set_module("boost_histogram.axis") | |||
| @register({ca.category_int, ca.category_int_growth}) | |||
| class IntCategory(BaseCategory): | |||
| @inject_signature("self, categories, *, metadata=None, growth=False") | |||
| __slots__ = () | |||
There was a problem hiding this comment.
This was a bug (missing __slots__). But it wouldn’t be anymore!
|
@HDembinski You've already agreed to the basic idea, so I'll merge at some point if I need to to work on the next PR without diverging too much - but feel free to let me know if I need to change / pull / add anything even after merging if needed! |
|
I'm going to try to work on #476 today, and axis changes (if there are any) might interact with this. So merging! |
This removes the whole facade and just really makes this a
__dict__class. You can now manipulate dict directly, it's completely natural, and C++ just caries a pointer to it. If you need to clear the dict, set it, etc., you can.