kedro-plugins icon indicating copy to clipboard operation
kedro-plugins copied to clipboard

feat(datasets): Add CSVDataset to dask module

Open michaelsexton opened this issue 11 months ago • 6 comments

Description

To add the capability for CSV datasets to be used with Kedro.

Development notes

Copied the Parquet dataset implementation for Dask and reimplemented for CSV files. Has been tested in use of Kedro pipelines, but no test classes written yet.

Checklist

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

michaelsexton avatar Mar 25 '24 03:03 michaelsexton

Thank you for this PR @michaelsexton ! I see the PR is in draft, do you need any input from us or not yet?

astrojuanlu avatar Mar 25 '24 15:03 astrojuanlu

Hi @astrojuanlu I believe I have completed all the tasks necessary in draft, however the unit tests in the checks keep failing. I am getting an AssertionError in the unit-tests (ubuntu-latest, 3.10) / unit-tests check, however I can't replicate that locally.

michaelsexton avatar Mar 28 '24 04:03 michaelsexton

@michaelsexton I've fixed several issues that weren't related to your changes, but it looks like the tests you added aren't passing currently. I can replicate this locally as well. Do you have some time to fix them?

merelcht avatar Apr 05 '24 14:04 merelcht

@merelcht Thanks, I should have some time later this week. Is it OK to discuss with you why they might failing if I continue running into them? Hopefully I'll get them all to pass anyway.

michaelsexton avatar Apr 08 '24 04:04 michaelsexton

@merelcht Thanks, I should have some time later this week. Is it OK to discuss with you why they might failing if I continue running into them? Hopefully I'll get them all to pass anyway.

Absolutely! I'm happy to help get this all working.

merelcht avatar Apr 08 '24 09:04 merelcht

Hi @michaelsexton just checking in to see if you're still interested in finishing this PR?

merelcht avatar May 15 '24 16:05 merelcht

Fixed the unit tests + linting! Made some small changes to the _exists() fn. I'll let other people from the team review this!

ankatiyar avatar Jun 12 '24 15:06 ankatiyar

We got 10 approvals already 💯

astrojuanlu avatar Jun 20 '24 10:06 astrojuanlu