Throw AttributeError for NIRNode constructor - #122
Conversation
Fix for neuromorphs#120 Throws `AttributeError` on call `NIRNode.__init__`.
|
Hi @hanleyc01! Thank you for your pull request (and your patience in our reply :-) )! This looks great, really. You're correct that this should throw if initialized, because it's not a valid computational primitive in NIR. All tests passed so I'll just go ahead and merge. Congratulations on your first contribution! Don't hesitate to join the discussion on Discord and come back with more changes ;-) |
Thanks so much! I'm glad to have helped, and I'm currently in the discord. I'm preparing for conference submissions with my lab until around the end of this month, but once I get more free time I'll try and get more active in the participation. |
Fix for #120 Throws `AttributeError` on call `NIRNode.__init__`.
Fix for #120
NIRNodefrom abc. This might be too much, but given that the stated documentation forNIRNodeis that it is a "base superclass" and that "NIRNodes should never be instantiated", indicating this property through inheritance in a Pythonic way.AttributeErroron callNIRNode.__init__as a proposed fix for Issue 120.test_nodewhich checks if aNIRNodeclass can be constructed. All tests pass, it seems.Potential Downsides
abc.ABCmight have some unintended consequences in behavior that I am not familiar with, this change I think can be discarded if there are any unintended problems that arise.NIRNode, they will find that__init__is in fact defined. However, the exception raised here is one used to indicate that there is in fact no__init__method. One potential alternative is to raiseTypeError, which "may be raised by user code to indicate that an attempted operation on an object is not supported, and is not meant to be".P.S. This is my first attempted contribution to an open-source library ❤️