-
Notifications
You must be signed in to change notification settings - Fork 901
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 deprecation warning for TemplatedConfigLoader
and ConfigLoader
#2840
Conversation
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
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! 🌟 Just left some minor suggestions, thank you @ankatiyar.
kedro/framework/session/session.py
Outdated
f"{type(config_loader).__name__} will be deprecated in Kedro 0.19." | ||
f" Please use the OmegaConfigLoader instead. To consult" | ||
f" the documentation for OmegaConfigLoader, see here:" | ||
f" https://docs.kedro.org/en/stable/configuration/advanced_configuration.html#omegaconfigloader", # pylint: disable=line-too-long |
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.
f"{type(config_loader).__name__} will be deprecated in Kedro 0.19." | |
f" Please use the OmegaConfigLoader instead. To consult" | |
f" the documentation for OmegaConfigLoader, see here:" | |
f" https://docs.kedro.org/en/stable/configuration/advanced_configuration.html#omegaconfigloader", # pylint: disable=line-too-long | |
f"{type(config_loader).__name__} will be deprecated in Kedro 0.19." | |
f" Please use the OmegaConfigLoader instead." | |
f" To import OmegaConfigLoader use: 'from kedro.config import OmegaConfigLoader'." | |
f" Consult the documentation for OmegaConfigLoader at:" | |
f" https://docs.kedro.org/en/stable/configuration/" | |
f"advanced_configuration.html#omegaconfigloader", |
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.
Minor suggestions (feel free to use or ignore):
- Maybe just split the link into multiple lines.
- Add the new import line for
OmegaConfigLoader
for convenience.
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.
Thanks Sajid, I split the link into multiple lines but I think the import line will make it too verbose. The link to the OmegaConfigLoader
docs has this right on top. :)
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 👍 Don't forget to add this to the release notes. This is an important deprecation that are users should be made aware of.
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
Description
Resolve #2693
Development notes
Added the deprecation warning in
session.py
when the context is loaded.Alternate Approach
__init__
functions ofConfigLoader
andTemplatedConfigLoader
but those warnings show up multiple times because the config loader is also initialised when loading logging then later when context is created.FutureWarning
instead ofDeprecationWarning
as it is visible by default (See Do not unsuppressDeprecationWarning
s with Kedro #2747 and Avoid setting warning filters globally #2744)Checklist
RELEASE.md
file