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

consider #NUM! error type as None #321

Merged
merged 3 commits into from
Jan 13, 2025
Merged

consider #NUM! error type as None #321

merged 3 commits into from
Jan 13, 2025

Conversation

sphuber
Copy link
Contributor

@sphuber sphuber commented Jan 10, 2025 •
edited
Loading

Fixes #320

@lukapeschke lukapeschke self-assigned this Jan 13, 2025
@lukapeschke lukapeschke added bug Something isn't working ✋ need review ✋ 🦀 rust 🦀 Pull requests that edit Rust code labels Jan 13, 2025
lukapeschke
lukapeschke requested changes Jan 13, 2025
View reviewed changes
Copy link
Collaborator

@lukapeschke lukapeschke left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your contribution, and welcome to fastexcel :)

The code LGTM, but looks like you'll have to adapt the test

python/tests/test_fastexcel.py Outdated Show resolved Hide resolved
@sphuber @lukapeschke
Update python/tests/test_fastexcel.py
5682861 
Co-authored-by: Luka Peschke <mail@lukapeschke.com>
@sphuber
Copy link
Contributor Author

sphuber commented Jan 13, 2025

Thanks a lot for your contribution, and welcome to fastexcel :)

Thank you

The code LGTM, but looks like you'll have to adapt the test

Accepted your suggestion, so let's see if that fixes the issue

lukapeschke
lukapeschke approved these changes Jan 13, 2025
View reviewed changes
Copy link
Collaborator

@lukapeschke lukapeschke left a comment

Choose a reason for hiding this comment

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

Tests passed, I'll merge asap and do a release soon

@sphuber
Copy link
Contributor Author

sphuber commented Jan 13, 2025

fantastic! thanks a lot for the quick handling

@lukapeschke lukapeschke merged commit 02a9b8a into ToucanToco:main Jan 13, 2025
26 checks passed
@lukapeschke
Copy link
Collaborator

@sphuber v0.12.1 is out and includes the fix 🎉 https://github.com/ToucanToco/fastexcel/releases/tag/v0.12.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lukapeschke lukapeschke lukapeschke approved these changes

Assignees

@lukapeschke lukapeschke

Labels
bug Something isn't working 🦀 rust 🦀 Pull requests that edit Rust code ✋ need review ✋
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Consider #NUM! errors values as None
2 participants
@sphuber @lukapeschke