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

dtype resolution is wrong for multi-dtype columns #160

Closed
lukapeschke opened this issue Jan 29, 2024 · 0 comments · Fixed by #164
Closed

dtype resolution is wrong for multi-dtype columns #160

lukapeschke opened this issue Jan 29, 2024 · 0 comments · Fixed by #164
Labels
bug Something isn't working 🦀 rust 🦀 Pull requests that edit Rust code
Milestone
v0.9.0

Comments

@lukapeschke
Copy link
Collaborator

Type coercion is currently done on the first cell of the column only. It should be done on the entire column. This needs to be addressed without impacting performance too much.

Originally posted by @lukapeschke in #158 (comment)
@lukapeschke lukapeschke added bug Something isn't working 🦀 rust 🦀 Pull requests that edit Rust code labels Jan 29, 2024
@lukapeschke lukapeschke mentioned this issue Jan 30, 2024
Merged
lukapeschke added a commit that referenced this issue Jan 31, 2024
@lukapeschke
feat(excelsheet): add support for multi-dtype columns
0f409cf 
closes #160

Signed-off-by: Luka Peschke <luka.peschke@toucantoco.com>
@lukapeschke lukapeschke mentioned this issue Jan 31, 2024
Merged
@danielcs88 danielcs88 mentioned this issue Feb 6, 2024
Closed
@PrettyWood PrettyWood added this to the v0.9.0 milestone Feb 7, 2024
@PrettyWood PrettyWood mentioned this issue Feb 7, 2024
Closed
lukapeschke added a commit that referenced this issue Feb 13, 2024
@lukapeschke @PrettyWood
feat(excelsheet): add support for multi-dtype columns (#164)
e243719 
* feat(deps-dev): as rstest as a dev dependency

Signed-off-by: Luka Peschke <luka.peschke@toucantoco.com>

* feat(excelsheet): add support for multi-dtype columns

closes #160

Signed-off-by: Luka Peschke <luka.peschke@toucantoco.com>

* fix: use as_f64 rather than get_float

Signed-off-by: Luka Peschke <luka.peschke@toucantoco.com>

* test: add null + int and null + int + float test case

Signed-off-by: Luka Peschke <luka.peschke@toucantoco.com>

* feat: add support for bools when determining the dtype fo a column

Signed-off-by: Luka Peschke <luka.peschke@toucantoco.com>

* feat: add support for int columns

Signed-off-by: Luka Peschke <luka.peschke@toucantoco.com>

* feat: added a schema_sample_rows param

Signed-off-by: Luka Peschke <luka.peschke@toucantoco.com>

* chore: doc

---------

Signed-off-by: Luka Peschke <luka.peschke@toucantoco.com>
Co-authored-by: Eric Jolibois <em.jolibois@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working 🦀 rust 🦀 Pull requests that edit Rust code
Projects
None yet
Milestone
v0.9.0
Development

Successfully merging a pull request may close this issue.

feat(excelsheet): add support for multi-dtype columns ToucanToco/fastexcel
2 participants
@lukapeschke @PrettyWood