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

Remove code for backwards compatibility #1254

Merged
merged 12 commits into from
Feb 21, 2022
Merged

Conversation

merelcht
Copy link
Member

@merelcht merelcht commented Feb 15, 2022

Signed-off-by: Merel Theisen merel.theisen@quantumblack.com

Description

We still had some code in place for backwards compatibility that should be removed in 0.18.0.

Development notes

  • Removed code in kedro.framework.project._init_.py there for backwards compatibility with projects that don't have the pipeline_registry.py file.
  • Removed configure_project(package_name) on session creation.

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: Merel Theisen <merel.theisen@quantumblack.com>
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
@merelcht merelcht force-pushed the KED-3021-tidy-project-init branch from 67c8088 to fe4f86a Compare February 15, 2022 16:03
@merelcht merelcht marked this pull request as ready for review February 15, 2022 16:22
@merelcht merelcht requested a review from idanov as a code owner February 15, 2022 16:22
@merelcht merelcht self-assigned this Feb 15, 2022
merelcht and others added 5 commits February 16, 2022 11:23
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
@merelcht merelcht force-pushed the KED-3021-tidy-project-init branch from 3846843 to bf33c5e Compare February 18, 2022 16:34
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
@merelcht merelcht force-pushed the KED-3021-tidy-project-init branch from bf33c5e to 42bcc4d Compare February 18, 2022 16:49
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
Copy link
Contributor

@antonymilne antonymilne left a comment

Choose a reason for hiding this comment

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

So much for this being a simple 1 point ticket 😬 Great job getting through it!

Just two small suggestions, not important:

  1. If _ProjectPipelines.configure is usually called with pipelines_module = None then it might be worth giving it a default value of None so those calls don't have to specify None explicitly (which looks very slightly unnatural to me).
  2. Maybe __init__ should also call self.configure just to reduce repetition and make it clear it's doing the same thing.

Copy link
Contributor

@lorenabalan lorenabalan left a comment

Choose a reason for hiding this comment

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

Awesome stuff! 🥇

Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
@merelcht
Copy link
Member Author

So much for this being a simple 1 point ticket 😬 Great job getting through it!

Just two small suggestions, not important:

  1. If _ProjectPipelines.configure is usually called with pipelines_module = None then it might be worth giving it a default value of None so those calls don't have to specify None explicitly (which looks very slightly unnatural to me).
  2. Maybe __init__ should also call self.configure just to reduce repetition and make it clear it's doing the same thing.

I've addressed point 1, but for point 2 it seems weird to not define the class attributes in __init__, and actually my IDE complains about that as well if I make the change.

@merelcht merelcht merged commit 326450b into develop Feb 21, 2022
@merelcht merelcht deleted the KED-3021-tidy-project-init branch February 21, 2022 10:59
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.

3 participants