-
Notifications
You must be signed in to change notification settings - Fork 3
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
Update interface of SigPro w/pipelines and primitives #54
Conversation
Made an additional slight modification to add a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great effort @andyx13! There are minor comments overall, and some improvements to be made to the pipeline initialization.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great @andyx13! Few comments to improve readability of the code.
I also noticed that the unittests are missing cases where the pipeline would raise an error. I think it would be good to include these cases as well!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ready to be merged! Great work @andyx13!
Larger update:
Primitive
andPipeline
base classes (Issue Update primitive interfaces #51)contributing_primitive
andbasic_primitives
module to assist with new primitive creation/usageAdditional modifications: