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.

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

ARROW-7796: [R] write_* functions should invisibly return their inputs #6387

Closed
wants to merge 11 commits into from
Closed

ARROW-7796: [R] write_* functions should invisibly return their inputs #6387

wants to merge 11 commits into from

Conversation

boshek
Copy link
Contributor

@boshek boshek commented Feb 7, 2020

I had some trouble building from source on Windows but I thought the changes were simple enough that I just went ahead. Tests and documentation also updated.

@github-actions
Copy link

github-actions bot commented Feb 7, 2020

Copy link
Member

@nealrichardson nealrichardson left a comment

Choose a reason for hiding this comment

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

Aside from a few apostrophes, LGTM. Could you please also add a bullet to the NEWS? Thanks!

r/tests/testthat/test-parquet.R Outdated Show resolved Hide resolved
r/tests/testthat/test-feather.R Outdated Show resolved Hide resolved
r/tests/testthat/test-read-write.R Outdated Show resolved Hide resolved
boshek and others added 5 commits February 7, 2020 13:39
Co-Authored-By: Neal Richardson <neal.p.richardson@gmail.com>
Co-Authored-By: Neal Richardson <neal.p.richardson@gmail.com>
Co-Authored-By: Neal Richardson <neal.p.richardson@gmail.com>
@boshek
Copy link
Contributor Author

boshek commented Feb 7, 2020

🤦‍♂ it's/its. Sorry about that. NEWS item added. Thanks for this great package!

r/NEWS.md Outdated Show resolved Hide resolved
Co-Authored-By: Neal Richardson <neal.p.richardson@gmail.com>
@nealrichardson
Copy link
Member

Thanks again!

nealrichardson pushed a commit to nealrichardson/arrow that referenced this pull request Feb 13, 2020
I had some trouble building from source on Windows but I thought the changes were simple enough that I just went ahead. Tests and documentation also updated.

Closes apache#6387 from boshek/write_return and squashes the following commits:

d4d1ead <Sam Albers> Update r/NEWS.md
5f0e316 <Sam Albers> Merge branch 'master' into write_return
5d32579 <Sam Albers> update NEWS
53c3ab9 <Sam Albers> Update r/tests/testthat/test-read-write.R
673de49 <Sam Albers> Update r/tests/testthat/test-feather.R
11db34c <Sam Albers> Update r/tests/testthat/test-parquet.R
12222d6 <Sam Albers> update documentation
3768d0f <Sam Albers> return invisibly
a595854 <Sam Albers> write_arrow returns input
cac8db8 <Sam Albers> write_feather returns input
f6b79de <Sam Albers> write_parquet returns input

Lead-authored-by: Sam Albers <sam.albers@gov.bc.ca>
Co-authored-by: Sam Albers <sam.albers@gmail.com>
Signed-off-by: Neal Richardson <neal.p.richardson@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants