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

Casts columns to string before checking for special characters #231

Merged
merged 5 commits into from
Jun 28, 2024
Merged

Casts columns to string before checking for special characters #231

merged 5 commits into from
Jun 28, 2024

Conversation

danmilne1
Copy link
Contributor

Fixes issue 227

@danmilne1 danmilne1 changed the base branch from main to dev June 28, 2024 10:39
@rowanhemsi rowanhemsi linked an issue Jun 28, 2024 that may be closed by this pull request
write_workbook errors with all integer table #227
Open
rowanhemsi
rowanhemsi requested changes Jun 28, 2024
View reviewed changes
Copy link
Collaborator

@rowanhemsi rowanhemsi left a comment

Choose a reason for hiding this comment

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

Looks like a sensible solution to an overlooked bug. Unfortunately the CI is failing on Windows with Python 3.6 - we no longer officially support 3.6 so if this can't be resolved that's ok, but please can you check to see if it can be first

gptables/test/test_wrappers.py
"table": pd.DataFrame({"a": [0], "b": [1]})
})

#Testing that this function executes with no errors
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for adding a comment to make this more explicit. Another option might be to use does_not_raise but for this situation that might be overkill

@rowanhemsi rowanhemsi self-requested a review June 28, 2024 13:52
rowanhemsi
rowanhemsi approved these changes Jun 28, 2024
View reviewed changes
Copy link
Collaborator

@rowanhemsi rowanhemsi left a comment •
edited
Loading

Choose a reason for hiding this comment

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

We have already discussed removing support for python 3.6, so I am happy to go with this if there is no obvious alternative solution, just make sure it's documented in the changelog. Thanks for getting this fixed

@danmilne1 danmilne1 merged commit 8a368ea into dev Jun 28, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rowanhemsi rowanhemsi rowanhemsi approved these changes

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

Successfully merging this pull request may close these issues.

write_workbook errors with all integer table
2 participants
@danmilne1 @rowanhemsi