-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: allow overriding of protocol methods in subclasses #128
feat: allow overriding of protocol methods in subclasses #128
Conversation
I personally think that indented control structures are more clear than imperative-jumping logic like
Nevermind: that's what you're doing already. I was misled by the "return -1 if not handler is found"—that can't happen because So if you think this PR is ready to merge, I am. (I'd only change the docstring of |
Thanks! Oh, if you consider this PR to be done, let me know and I'll merge it after the tests pass. |
You are completely right: if the I addressed your comment on 9c60f5f, and I think it is ready to merge. |
Just checking: would these changes produce a new minor version release? or you guys have other time-frame in mind? I am asking because the current MadMiner approach to the subclassing is broken, and I would like to know if we should create a temporal solution, or just wait for... -check notes- |
I think we can push a new patch release as soon as this is merged. |
Co-authored-by: Henry Schreiner <HenrySchreinerIII@gmail.com>
for more information, see https://pre-commit.ci
This PR addresses issue #127 by extending the _handler_of function to consider the module of all the inherited classes of a certain object. This is neccesary to allow the definition of client-side custom classes that can inherit from the library ones, and override their protocol methods (
__iadd__
,__isub__
...).Additional clarifications
if isinstance(obj, Vector)
to reduce indentation.contextlib.suppress
helper to reduce indentation.cc @jpivarski