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

fix(datasets): Fix polars.CSVDataset save on Windows #979

Merged
merged 9 commits into from
Jan 13, 2025
Merged

fix(datasets): Fix polars.CSVDataset save on Windows #979

merged 9 commits into from
Jan 13, 2025

Conversation

ravi-kumar-pilla
Copy link
Contributor

@ravi-kumar-pilla ravi-kumar-pilla commented Jan 9, 2025 •
edited
Loading

Description

Resolves #959

Development notes

  • Added default encoding in open_args_save.
  • Removed xfail for win tests
  • Update release note

Checklist

  • Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • Updated the documentation to reflect the code changes
  • Updated jsonschema/kedro-catalog-X.XX.json if necessary
  • Added a description of this change in the relevant RELEASE.md file
  • Added tests to cover my changes
  • Received approvals from at least half of the TSC (required for adding a new, non-experimental dataset)

@ravi-kumar-pilla ravi-kumar-pilla changed the title Fix polars.CSVDataset on Windows fix(datasets): Fix polars.CSVDataset save on Windows Jan 9, 2025
@ravi-kumar-pilla ravi-kumar-pilla marked this pull request as ready for review January 9, 2025 23:07
@ravi-kumar-pilla
Copy link
Contributor Author

ravi-kumar-pilla commented Jan 9, 2025 •
edited
Loading

Hi @deepyaman ,
The PR resolves the bug but I could not find the root cause of what exactly made the bug with polars 1.x. Tested in windows and save fails with the encoding error. Thank you

deepyaman
deepyaman approved these changes Jan 13, 2025
View reviewed changes
Copy link
Member

@deepyaman deepyaman left a comment

Choose a reason for hiding this comment

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

Simple and clean.

merelcht
merelcht approved these changes Jan 13, 2025
View reviewed changes
Copy link
Member

@merelcht merelcht left a comment

Choose a reason for hiding this comment

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

🎉

@ravi-kumar-pilla ravi-kumar-pilla merged commit 630f4ea into main Jan 13, 2025
12 of 13 checks passed
@ravi-kumar-pilla ravi-kumar-pilla deleted the fix/polars-csv-win-tests branch January 13, 2025 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@deepyaman deepyaman deepyaman approved these changes

@merelcht merelcht merelcht approved these changes

@DimedS DimedS Awaiting requested review from DimedS

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

Successfully merging this pull request may close these issues.

bug(datasets): fix polars.CSVDataset Windows tests
3 participants
@ravi-kumar-pilla @deepyaman @merelcht