Restore heeding option datatable.showProgress if set

[HughParsonage]

Matt, I wasn't sure whether showProgress must be FALSE if interactive() is also FALSE

[jangorecki]

Why not

getOption("datatable.showProgress", interactive())

I used that option sometimes, only to FALSE :)

[codecov-io]

Codecov Report

Merging #2785 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2785   +/-   ##
=======================================
  Coverage   93.49%   93.49%           
=======================================
  Files          61       61           
  Lines       12367    12367           
=======================================
  Hits        11562    11562           
  Misses        805      805

Impacted Files	Coverage Δ
R/fwrite.R	100% <ø> (ø)	⬆️
R/fread.R	96.87% <ø> (ø)	⬆️

---

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 3deeb38...083d7be. Read the comment docs.

[HughParsonage]

@jangorecki Yes, that might be better. I just wasn't sure how deliberate the decision to remove the option was, or whether interactive() should trump the option. I seem to remember this change caused quite a lot of difficulty, so I wanted to keep the change minimal with respect to the current dev.

[MichaelChirico]

are you able to find the commit/issue in which it was changed? for citation purposes

…

On Mon, Apr 23, 2018, 12:45 PM HughParsonage ***@***.***> wrote: @jangorecki <https://github.com/jangorecki> Yes, that might be better. I just wasn't sure how deliberate the decision to remove the option was, or whether interactive() should trump the option. I seem to remember this change caused quite a lot of difficulty, so I wanted to keep the change minimal with respect to the current dev. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2785 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHQQdYEkEsmE-F8FX7wb30F1UouVVss_ks5trVxpgaJpZM4TfOtU> .

[HughParsonage]

6342551

[MichaelChirico]

Hmm, I don't see anything there or in linked #2070 suggesting the need for the argument change

[bogind]

I just uploaded a fix to the easycsv functions that use fread.
It should work even after the update.
if it won't you may feel free to contribute there, easycsv is relatively small and simple and as far as i'm concerned any help is welcome.

[mattdowle]

Thanks all for efforts here. I don't recall why I dropped that option. And as you found too, I can't see anything in the history either. Did I change showProgress default around the same time that I (erroneously) thought that the fread multi-threaded crash in RStudio was to do with the progress bar? Anyway, @jangorecki's suggestion looks best to me. I may have been motivated at the time to remove options because of the overhead of getOption() when called a lot in a loop. But that's not a concern for fread/fwrite and also, more recent efforts to improve getOption speed in R itself have been largely successful. So the default argument being a call to getOption including default=interactive() directly there is clearest and most helpful to users. I don't anticipate any disagreement given the comments above so I'll change now and merge.

[mattdowle]

With my change just now, current CRAN version of easycsv will break again. Because it calls getOption("datatable.showProgress") which returns NULL and I've taken away the branch Hugh added to accept NULL. I've done this because @bogind has kindly written above that they have changed easycsv. Changing easycsv in this case is the best thing to do because data.table is unusual for setting options to a default value when data.table loads, so packages should not assume that by using getOption() without passing default=. This brings us into line with correct and common usage of getOption(). The main thing is that args(fread) and ?fread are now very clear how showProgress is set and what the default is. The argument and its default are checked by R CMD check to be consistent in fread.R and fread.Rd, too.
The easycsv update is unlikely to be accepted by CRAN until data.table has been updated there first. What I usually do is provide CRAN a list of packages that will break and let them know communication has happened and that those maintainers are prepared and ok to submit their updates afterwards. I hope this is ok @bogind. Many thanks.

[mattdowle]

Same applies for package SIRItoGTFS, if that's ok with you @bogind?

[bogind]

@mattdowle fine with me.
it will get a fix too in the next couple of days.