Skip to content

ARROW-5504 [R]: move use_threads argument to global option#4515

Closed
romainfrancois wants to merge 2 commits intoapache:masterfrom
romainfrancois:ARROW-5504/use_threads
Closed

ARROW-5504 [R]: move use_threads argument to global option#4515
romainfrancois wants to merge 2 commits intoapache:masterfrom
romainfrancois:ARROW-5504/use_threads

Conversation

@romainfrancois
Copy link
Contributor

@romainfrancois romainfrancois commented Jun 11, 2019

At this point the function is not exported or documented and threads are always used, users would need to set options(arrow.use_threads) to turn them off.

@codecov-io
Copy link

codecov-io commented Jun 11, 2019

Codecov Report

Merging #4515 into master will decrease coverage by 13.43%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #4515       +/-   ##
===========================================
- Coverage   88.11%   74.67%   -13.44%     
===========================================
  Files         850       54      -796     
  Lines      105774     3112   -102662     
  Branches     1253        0     -1253     
===========================================
- Hits        93203     2324    -90879     
+ Misses      12326      788    -11538     
+ Partials      245        0      -245
Impacted Files Coverage Δ
r/R/R6.R 75% <ø> (ø) ⬆️
r/R/ChunkedArray.R 100% <ø> (ø) ⬆️
r/R/array.R 76% <ø> (ø) ⬆️
r/src/arrowExports.cpp 72.7% <ø> (ø) ⬆️
r/R/read_table.R 100% <ø> (ø) ⬆️
r/R/feather.R 58.33% <100%> (ø) ⬆️
r/R/Table.R 86.66% <100%> (ø) ⬆️
r/R/csv.R 95.45% <100%> (ø) ⬆️
r/R/parquet.R 100% <100%> (ø) ⬆️
r/R/arrow-package.R 100% <100%> (ø)
... and 798 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 48ee38f...cfa7488. Read the comment docs.

Copy link
Member

Choose a reason for hiding this comment

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

Let's keep this test so that we're exercising the single-threaded code and ensuring equality. Just have to change the use_threads=FALSE to flip the option instead.

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.

One note about testing but otherwise LGTM

@romainfrancois romainfrancois force-pushed the ARROW-5504/use_threads branch 2 times, most recently from cfa7488 to ae8dae2 Compare June 12, 2019 11:30
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