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

[r] Update write_soma.data.frame() to support categoricals #1686

Merged
merged 3 commits into from
Sep 17, 2023
Merged

[r] Update write_soma.data.frame() to support categoricals #1686

merged 3 commits into from
Sep 17, 2023

Conversation

mojaveazure
Copy link
Member

@mojaveazure mojaveazure commented Sep 16, 2023 •
edited
Loading

Allow write_soma.data.frame() to write factors as categoricals. write_soma() will no longer cast factors to characters prior to writing
Redo of #1683 since it got removed from #1559

@mojaveazure
[r] Update write_soma.data.frame() to support categoricals
d2d0a8d 
Allow `write_soma.data.frame()` to write factors as categoricals. `write_soma()` will no longer cast factors to characters prior to writing
Redo of #1683 since it got removed from #1559
@codecov-commenter
Copy link

codecov-commenter commented Sep 16, 2023 •
edited
Loading

Codecov Report

Patch has no changes to coverable lines.

❗ Current head 06744b1 differs from pull request most recent head 8a2c879. Consider uploading reports for the commit 8a2c879 to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

📢 Thoughts on this report? Let us know!.

eddelbuettel
eddelbuettel approved these changes Sep 16, 2023
View reviewed changes
Copy link
Contributor

@eddelbuettel eddelbuettel left a comment

Choose a reason for hiding this comment

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

Should we add a simple test for round-turn?

aaronwolen
aaronwolen approved these changes Sep 16, 2023
View reviewed changes
Copy link
Member

@aaronwolen aaronwolen left a comment

Choose a reason for hiding this comment

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

Thank you! I'll second Dirk's request for a unit test but otherwise looks good to go.

johnkerl
johnkerl requested changes Sep 16, 2023
View reviewed changes
Copy link
Member

@johnkerl johnkerl left a comment

Choose a reason for hiding this comment

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

Let's get the unit-test case in

johnkerl
johnkerl approved these changes Sep 17, 2023
View reviewed changes
Copy link
Member

@johnkerl johnkerl left a comment

Choose a reason for hiding this comment

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

🚢
Thanks @mojaveazure !!

@johnkerl johnkerl merged commit 355372d into main Sep 17, 2023
@johnkerl johnkerl deleted the ph/feat/write-soma-categoricals2 branch September 17, 2023 00:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@johnkerl johnkerl johnkerl approved these changes

@aaronwolen aaronwolen aaronwolen approved these changes

@eddelbuettel eddelbuettel eddelbuettel approved these changes

Assignees
No one assigned
Labels
r-api
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@mojaveazure @codecov-commenter @eddelbuettel @aaronwolen @johnkerl