Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
To string with encoding #28951
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.
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
To string with encoding #28951
Changes from all commits
ae76e46
3985040
8eea18e
d2c70ee
835b22d
4e30d7c
78ba34b
56fdad9
83ccffd
19801bb
d44afa7
b4b983b
acb2c6e
236db38
93e8b11
1f3e55d
b0364d2
12ddc8f
f82fe78
5ef9cec
df58c1f
496f68f
725b2ec
698d3b7
96cc810
1a35eeb
f865ac3
757a1f5
835cdb8
648fa55
3698a2a
f8917de
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can remove the invalid encoding; this doesn't test any function pandas provides rather just builtin Python functionality
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@simonjayhawkins thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason I asked for it to be added was so that the precedence of the Exceptions was checked and to confirm the encoding parameter was passed to the builtin function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh OK my mistake - just didn't see that was asked for previously (lost in GH comments)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so agree with #28951 (comment) but this sort of compensates. If we conform the encoding is passed, then reading back in is only testing Python functionality.
float_frame will probably work with any encoding, so maybe best to modify float_frame if encoding=="gbq".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should work...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason for this to be a fixture instead of just a global function? This way of invoking the function seems very magical; I think easier if not a fixture
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall I make this change as a part of this PR itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm sorry thought it was a part of this PR. I think OK to do here but let's see what @simonjayhawkins thinks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is fine as is.