Skip to content
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

Merged
merged 4 commits into from
Jul 27, 2023

Conversation

ankatiyar
Copy link
Contributor

Description

Resolve #2693

Development notes

Added the deprecation warning in session.py when the context is loaded.

Alternate Approach

Checklist

  • Read the contributing guidelines
  • Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • Updated the documentation to reflect the code changes
  • Added a description of this change in the RELEASE.md file
  • Added tests to cover my changes

Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
Copy link
Contributor

@SajidAlamQB SajidAlamQB left a 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.

Comment on lines 268 to 271
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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",

Copy link
Contributor

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.

Copy link
Contributor Author

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. :)

Copy link
Member

@merelcht merelcht left a 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.

@ankatiyar ankatiyar enabled auto-merge (squash) July 27, 2023 09:40
@ankatiyar ankatiyar merged commit 3a02aa4 into main Jul 27, 2023
20 of 21 checks passed
@ankatiyar ankatiyar deleted the depracation-warnings branch July 27, 2023 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add deprecation warnings to ConfigLoader and TemplatedConfigLoader
3 participants