-
Notifications
You must be signed in to change notification settings - Fork 611
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
Public function to register custom ops #1193
Conversation
Haven't had time for a full review, but at first glance looks like a good strategy. @guillaumekln could you take a look when time allows and see if this covers the issue from your perspective. |
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.
This looks good to me, thanks for working on this issue!
Thanks a lot for the review! Let me fix all that. |
@seanpmorgan Do you want to take a second look before merging? |
This pull request will be discussed at the meeting tonight I believe. |
@gabrieldemarmiesse Conflicts and then LGTM. Per discussion we'll want to work toward registering Keras objects in a similar manner |
Let's do that in another pull request to make the review easier. |
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.
LGTM Thanks!
* Added public functions to register everything. * Removed decorator * Revert "Removed decorator" This reverts commit ebea5bd. * Added some tests. * Added the two register. * Removed unused variables. * Private func. * Explicit modules. * FLake8 * Added documentation. * Remove useless setup method. * Black/ * Format BUILD.
Fix #1151
I'll open an issue to track the registering of keras objects with this function. It's there for future backward compatibility.