Skip to content

ARROW-3941: [R] RecordBatchStreamReader$schema - #3104

Closed
romainfrancois wants to merge 2 commits into
apache:masterfrom
romainfrancois:ARROW-3941/RecordBatchStreamReader
Closed

ARROW-3941: [R] RecordBatchStreamReader$schema#3104
romainfrancois wants to merge 2 commits into
apache:masterfrom
romainfrancois:ARROW-3941/RecordBatchStreamReader

Conversation

@romainfrancois

Copy link
Copy Markdown
Contributor

https://issues.apache.org/jira/browse/ARROW-3941

follow up to #3043 to fix api of classes RecordBatchFileReader, RecordBatchStreamReader, RecordBatchFileWriter, RecordBatchStreamWriter

@codecov-io

Copy link
Copy Markdown

Codecov Report

Merging #3104 into master will increase coverage by 1.1%.
The diff coverage is n/a.

Impacted file tree graph

@@ Coverage Diff @@## master #3104 +/- ##
=========================================
+ Coverage 87.04% 88.15% +1.1% 
=========================================
Files 492 434 -58 Lines 69104 65339 -3765 =========================================
- Hits 60154 57601 -2553 + Misses 8849 7738 -1111 + Partials 101 0 -101
Impacted FilesCoverage Δ
cpp/src/plasma/thirdparty/ae/ae.c72.03% <0%> (-0.95%)⬇️
cpp/src/arrow/util/thread-pool-test.cc98.91% <0%> (-0.55%)⬇️
go/arrow/array/table.go
go/arrow/math/uint64_amd64.go
go/arrow/internal/testing/tools/bool.go
go/arrow/internal/bitutil/bitutil.go
go/arrow/memory/memory_avx2_amd64.go
go/arrow/array/null.go
go/arrow/datatype_nested.go
go/arrow/array/string.go
... and 50 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0afb975...01bd167. Read the comment docs.

wesm
wesm approved these changes Dec 5, 2018

@wesmwesm 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.

+1, but see comment re: removing deprecated API

Comment threadr/R/RecordBatchReader.R Outdated
public = list(
schema = function() shared_ptr(`arrow::Schema`, RecordBatchReader__schema(self)),
ReadNext = function() {
get_next_batch = function() {

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.

get_next_batch is deprecated, can you remove?

expect_is(batch1, "arrow::RecordBatch")
expect_equal(batch, batch1)

expect_null(reader$get_next_batch())

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.

s/get/read/g

@wesm

wesm commented Dec 6, 2018

Copy link
Copy Markdown
Member

Test failure was transient. Merging

@wesmwesm closed this in a084006Dec 6, 2018
@romainfrancois
romainfrancois deleted the ARROW-3941/RecordBatchStreamReader branch December 7, 2018 13:07
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@romainfrancois@codecov-io@wesm