-
Notifications
You must be signed in to change notification settings - Fork 72
Automatic Serialization #323
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
Conversation
That's a super nice utility! Which networks are already tested through? |
It's still in progress, so not functional for all networks yet. But I made changes such that most networks should already be tested. |
This looks really nice! Please take care when removing |
b5c5369
to
e2304e7
Compare
@vpratz Thank you for raising the issue. After investigating further, I opted to go with a semi-automatic and more classical OOP approach of using a |
Thanks for taking this into account. I currently don't have a clear opinion, so I'll just share some thoughts:
|
Thanks for sharing. To address your points:
|
Thanks a lot for the detailed response. With this additional information on the constraints, I'm happy with the proposed solution. |
* add numpy transform * remove lambda transform * repurpose `adapter.apply()` for `NumpyTransform` instead of `LambdaTransform` * fix usages of `adapter.apply()` in example notebooks * fix tests * only allow strings as arguments (subject to be fixed by #323) * unify serialization pattern * remove old lambda transform error message * add fail fast to CI tests * cannot suppport filtering kwargs for numpy ufunc in python 3.10
Closing this as the better approach seems to be to fix our implementations of the |
The goals of this PR are:
from_config
andget_config
methods for most objectsconfig
object in the constructor of most objectsTo achieve the above goals, this PR introduces:
serialize
anddeserialize
function that wrap around thekeras
utilitiesSerializable
class that automatically implementsfrom_config
andget_config
Fixes #342