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

Fix issues with 1.7.1 kedro-datasets release #1558

Closed
ravi-kumar-pilla opened this issue Oct 6, 2023 · 7 comments · Fixed by #1559
Closed

Fix issues with 1.7.1 kedro-datasets release #1558

ravi-kumar-pilla opened this issue Oct 6, 2023 · 7 comments · Fixed by #1559
Assignees
Labels
blocked This PR needs is waiting for one or more issues to be closed. Dependencies Pull requests that update a dependency file

Comments

@ravi-kumar-pilla
Copy link
Contributor

ravi-kumar-pilla commented Oct 6, 2023

With Kedro Datasets 1.7.1 release , we need to update the dataset imports and fix pytests

We will be pinning down the dependency of kedro datasets to 1.7.0 as a quick fix

@ravi-kumar-pilla ravi-kumar-pilla self-assigned this Oct 6, 2023
@ravi-kumar-pilla ravi-kumar-pilla added Dependencies Pull requests that update a dependency file blocked This PR needs is waiting for one or more issues to be closed. labels Oct 6, 2023
@astrojuanlu
Copy link
Member

Hi @ravi-kumar-pilla , what broke? Do you have a URL of a failing build?

@ravi-kumar-pilla
Copy link
Contributor Author

Hi @ravi-kumar-pilla , what broke? Do you have a URL of a failing build?

Hi Juan, The circle ci builds are failing - https://app.circleci.com/pipelines/github/kedro-org/kedro-viz/10632/workflows/a8d8d61d-132a-4a48-8990-1ffa51bd422b/jobs/60837

@astrojuanlu
Copy link
Member

I see - if I understand correctly, technically yeah this is a breaking change from 1.7.0 to 1.7.1 right? so maybe there's something to be done on the datasets side. cc @ankatiyar @merelcht

@ankatiyar
Copy link
Contributor

Changing DataSet -> Dataset in the tests should fix this I think

@merelcht
Copy link
Member

merelcht commented Oct 6, 2023

Yes that would fix it. I'm surprised that the tests started failing, because the aliases are still in place.. It might be because of hardcoded DataSet in strings though..

@ankatiyar
Copy link
Contributor

Yeah, you can still create datasets with CSVDataSet(filepath) but the type of the dataset created would be CSVDataset and it's the assertions against hardcoded strings in the tests that are failing ->

assert transcoded_data_node_metadata.transcoded_types == [
            "pandas.csv_dataset.CSVDataSet"
        ]

@ravi-kumar-pilla
Copy link
Contributor Author

Thank you for the suggestions. As a quick fix, we decided to pin down kedro-datasets version to 1.7 in the requirements file. We will be adding support for kedro datasets 1.7.1 and above in the upcoming sprints. Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked This PR needs is waiting for one or more issues to be closed. Dependencies Pull requests that update a dependency file
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants