Skip to content

ARROW-11756: [R] passing a partition as a schema leads to segfaults#9566

Closed
pachadotdev wants to merge 7 commits into
apache:masterfrom
pachadotdev:master
Closed

ARROW-11756: [R] passing a partition as a schema leads to segfaults#9566
pachadotdev wants to merge 7 commits into
apache:masterfrom
pachadotdev:master

Conversation

@pachadotdev

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread r/R/dataset.R Outdated

@jonkeane jonkeane left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good work! A few suggestions for you to consider.

Comment thread r/tests/testthat/test-dataset.R Outdated
@github-actions

Copy link
Copy Markdown

@pachadotdev

Copy link
Copy Markdown
Contributor Author

@jonkeane new PR changes in #fed2fd8, which passes verbose error message
I see it passes the gh-actions checks

@jonkeane jonkeane left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks good to me with a slightly edit to the actual message. Great work!

Comment thread r/R/dataset.R Outdated
@pachadotdev

Copy link
Copy Markdown
Contributor Author

This looks good to me with a slightly edit to the actual message. Great work!

sure ! updated now

@nealrichardson nealrichardson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks! A few suggestions.

Comment thread r/R/dataset.R Outdated
Comment thread r/tests/testthat/test-dataset.R Outdated
Comment thread r/tests/testthat/test-dataset.R Outdated
@pachadotdev

Copy link
Copy Markdown
Contributor Author

@nealrichardson @jonkeane all ready, I've added all the comments
I think I start to understand the coding style used internally

@nealrichardson nealrichardson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

One last change but otherwise looks good, thanks!

Comment thread r/R/dataset.R Outdated

@nealrichardson nealrichardson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants