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.

Sign up for GitHub

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

Jump to bottom

Add type alias for dataset factory patterns #2779

Merged
merged 5 commits into from
Jul 7, 2023
Merged

Add type alias for dataset factory patterns #2779

merged 5 commits into from
Jul 7, 2023

Conversation

ankatiyar
Copy link
Contributor

NOTE: Kedro datasets are moving from kedro.extras.datasets to a separate kedro-datasets package in
kedro-plugins repository. Any changes to the dataset implementations
should be done by opening a pull request in that repository.

Description

Related to #2776

Including @noklam's suggestion about using type alias for patterns and @idanov's suggestions on the merged PR for dataset factories #2635

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

@ankatiyar
Add type alias for patterns and refactoring
ab77793 
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
@ankatiyar ankatiyar requested a review from merelcht as a code owner July 7, 2023 11:11
@ankatiyar ankatiyar requested review from idanov and noklam July 7, 2023 11:11
idanov
idanov reviewed Jul 7, 2023
View reviewed changes
kedro/io/data_catalog.py Outdated Show resolved Hide resolved
ankatiyar and others added 2 commits July 7, 2023 12:23
@ankatiyar @idanov
Update kedro/io/data_catalog.py
3cb4458 
Co-authored-by: Ivan Danov <idanov@users.noreply.github.com>
@ankatiyar
Merge branch 'main' into fix/type-aliasing-factories
4bf112f
idanov
idanov approved these changes Jul 7, 2023
View reviewed changes
Copy link
Member

@idanov idanov left a comment

Choose a reason for hiding this comment

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

⭐️

noklam
noklam approved these changes Jul 7, 2023
View reviewed changes
Copy link
Contributor

@noklam noklam left a comment

Choose a reason for hiding this comment

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

Looks good! It's more clean and easier to read!

ankatiyar and others added 2 commits July 7, 2023 15:30
@ankatiyar
Also add type alias to init fn
8a18bbe 
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
@ankatiyar
Merge branch 'main' into fix/type-aliasing-factories
dbb2b3c
@ankatiyar ankatiyar enabled auto-merge (squash) July 7, 2023 15:21
@ankatiyar ankatiyar merged commit 26057fc into main Jul 7, 2023
@ankatiyar ankatiyar deleted the fix/type-aliasing-factories branch July 7, 2023 15:34
@noklam noklam mentioned this pull request Jul 31, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@noklam noklam noklam approved these changes

@idanov idanov idanov approved these changes

@merelcht merelcht Awaiting requested review from merelcht merelcht is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ankatiyar @idanov @noklam