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

GH-36271: [R] Split out R6 classes and convenience functions #36394

Merged
merged 12 commits into from
Jul 11, 2023
Merged

GH-36271: [R] Split out R6 classes and convenience functions #36394

merged 12 commits into from
Jul 11, 2023

Conversation

thisisnic
Copy link
Member

@thisisnic thisisnic commented Jun 29, 2023 •
edited by github-actions bot
Loading

@thisisnic
Copy link
Member Author

@github-actions crossbow submit preview-docs

@github-actions
Copy link

Revision: 439e2a8

Submitted crossbow builds: ursacomputing/crossbow @ actions-c1ef6de2e5

Task Status
preview-docs Github Actions

@thisisnic
Copy link
Member Author

@github-actions crossbow submit preview-docs

@github-actions
Copy link

Revision: bcb04d8

Submitted crossbow builds: ursacomputing/crossbow @ actions-43f1b60cff

Task Status
preview-docs Github Actions

@thisisnic
Copy link
Member Author

@github-actions crossbow submit preview-docs

@thisisnic thisisnic marked this pull request as ready for review July 10, 2023 13:49
@github-actions
Copy link

Revision: a9745c2

Submitted crossbow builds: ursacomputing/crossbow @ actions-4747ca0561

Task Status
preview-docs Github Actions

@github-actions
Copy link

Revision: 5bfd1bb

Submitted crossbow builds: ursacomputing/crossbow @ actions-c28dae260a

Task Status
preview-docs Github Actions

paleolimbot
paleolimbot approved these changes Jul 10, 2023
View reviewed changes
Copy link
Member

@paleolimbot paleolimbot left a comment

Choose a reason for hiding this comment

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

This looks great! Thank you!

When browsing the preview docs, the only possible improvement related to this change might be updating:

- title: Arrow data types, schemas, and containers - R6 classes
  desc: >
    R6 classes for Arrow data types.

to something like

- title: Low-level C++ wrappers
  desc: >
    Low-level R6 class representations of Arrow C++ objects intended for advanced users.

...with as much editorial take-it-or-leave-it as you see fit!

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting committer review Awaiting committer review labels Jul 10, 2023
@thisisnic
Copy link
Member Author

Low-level R6 class representations of Arrow C++ objects intended for advanced users.

This is a fantastic improvement, thanks @paleolimbot, have integrated it!

@thisisnic thisisnic merged commit 2bd031a into apache:main Jul 11, 2023
raulcd pushed a commit that referenced this pull request Jul 13, 2023
@thisisnic @raulcd
GH-36271: [R] Split out R6 classes and convenience functions (#36394)
dc6954a 
Closes: #36271 
* Closes: #36271

Authored-by: Nic Crane <[email protected]>
Signed-off-by: Nic Crane <[email protected]>
@conbench-apache-arrow
Copy link

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 2bd031a.

There were 3 benchmark results indicating a performance regression:

The full Conbench report has more details. It also includes information about possible false positives for unstable benchmarks that are known to sometimes produce them.

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

@paleolimbot paleolimbot paleolimbot approved these changes

Assignees
No one assigned
Labels
awaiting merge Awaiting merge Component: R
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[R] Split out R6 classes and convenience functions
2 participants
@thisisnic @paleolimbot