-
Notifications
You must be signed in to change notification settings - Fork 905
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 PartitionedDataset and IncrementalDataset from kedro.io #3187
Conversation
Signed-off-by: SajidAlamQB <[email protected]>
Signed-off-by: SajidAlamQB <[email protected]>
Signed-off-by: SajidAlamQB <[email protected]>
This needs a |
Is this going into kedro-org/kedro-plugins#388 or 2.0? |
Isn't it going into 1.8.0? kedro-org/kedro-plugins#253 |
Yes it's going into |
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.
All good from my perspective. I adjusted a couple of words for Vale feedback but we can ignore most of the whining because there's no easier/better way to explain it.
Signed-off-by: SajidAlamQB <[email protected]>
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.
Don't forget to add this to the release notes! Kedro datasets 1.8.0
is released, so this should be good to go.
Signed-off-by: SajidAlamQB <[email protected]>
Signed-off-by: SajidAlamQB <[email protected]>
Signed-off-by: Sajid Alam <[email protected]>
Signed-off-by: SajidAlamQB <[email protected]>
Signed-off-by: SajidAlamQB <[email protected]>
Signed-off-by: Sajid Alam <[email protected]>
Signed-off-by: SajidAlamQB <[email protected]>
…m/kedro-org/kedro into remove-partitoned-and-incremental Signed-off-by: SajidAlamQB <[email protected]> Signed-off-by: SajidAlamQB <[email protected]>
Signed-off-by: SajidAlamQB <[email protected]>
Signed-off-by: SajidAlamQB <[email protected]>
Ugh:
This is because of |
The build over there passed https://readthedocs.org/projects/kedro-datasets/builds/22330550/ |
Okay this is why: The RTD project for
It turns out the new datasets are using the old, deprecated names: This was an oversight while reviewing kedro-org/kedro-plugins#350 ✋🏽 The best course of action, I think, is to not have those datasets in the docs yet, move forward with this PR, and in parallel make a fix on kedro-datasets and release a 1.8.1 soon-ish. |
Signed-off-by: SajidAlamQB <[email protected]>
We'll have to do that ASAP then, because the whole reason of getting |
The bugfix can maybe go directly into 2.0.0, depending on how much time do we intend to wait for that one. As long as it's release before Kedro 0.19.0... |
Description
We migrated PartitionedDataset and IncrementalDataset to kedro-datasets, kedro-org/kedro-plugins#253, so we can remove them from framework.
Development notes
Updated
test_data_catalog.py
to test theconfrim
method using the moved IncrementalDataset.Developer Certificate of Origin
We need all contributions to comply with the Developer Certificate of Origin (DCO). All commits must be signed off by including a
Signed-off-by
line in the commit message. See our wiki for guidance.If your PR is blocked due to unsigned commits, then you must follow the instructions under "Rebase the branch" on the GitHub Checks page for your PR. This will retroactively add the sign-off to all unsigned commits and allow the DCO check to pass.
Checklist
RELEASE.md
file