-
Notifications
You must be signed in to change notification settings - Fork 96
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
Add meta class to handle config saving #591
Add meta class to handle config saving #591
Conversation
…cation warning. To not break backwards compatibility.
Previously mentioned bug was resolved by #593, wasn't an issue with this PR. |
mypy is only failing for code, which wasn't modified by this PR. |
@AMHermansen Let's take a look at this together after the dev meeting, if you have time. |
@RasmusOrsoe I've now changed the typehints from the source code, that was conflicting with mypy (except for the SQLitePertubedDataset). This means that currently it is only the examples which are failing for mypy, feel free to have a look at this PR when you get the time 😄 . |
Currently we're using a decorator around every
__init__
of classes that inherits fromModel/Dataset
. This creates redundant boilerplate code. This PR aims to introduce a new way of handling the backend config saving. This is done by creating metaclasses responsible for intercepting the objects created and store a corresponding config dict to the created objects.The following is done:
ModelConfigSaverMeta
,DatasetConfigSaverMeta
) along with common interfaces between these metaclasses andABCMeta
.save_model_config
andsave_dataset_config
and add deprecation warnings when they're used in code.This means the config saving is purely done in the backend, away from any frontend users, making the code easier to read and avoids potential slip-ups where one might forget to use
@save_model_config
.This PR currently introduces a bug. I suspect the bug is a conflict with #584.
Could you maybe try to take a look at this @RasmusOrsoe