Rename showProgress argument to [

[MichaelChirico]

@jangorecki requested we reconsider the upperCamel naming style, which doesn't match anything else inside [ currently:

data.table/R/data.table.R

Line 130 in 6cee825

"[.data.table" = function(x, i, j, by, keyby, with=TRUE, nomatch=NA, mult="all", roll=FALSE, rollends=if (roll=="nearest") c(TRUE,TRUE) else if (roll>=0) c(FALSE,TRUE) else c(TRUE,FALSE), which=FALSE, .SDcols, verbose=getOption("datatable.verbose"), allow.cartesian=getOption("datatable.allow.cartesian"), drop=NULL, on=NULL, env=NULL, showProgress=getOption("datatable.showProgress", interactive()))

I put a poll with 4 options: showProgress, showprogress, show.progress, and show_progress; show.progress is preferred among those (3 days, 38+ responses).

A few respondents also suggested just plain progress= as another option -- any opinions on show.progress vs. progress? Personally I like show.progress as it makes it clearer this should be TRUE/FALSE. cc @joshhwuu as original PR author for vis.

[joshhwuu]

Happy to go with show.progress. However, we will have to consider how this argument handles options. Currently works with datatable.showProgress which is the same for fread/fwrite, but not sure if we would want to introduce a different option datatable.show.progress, keep the same option or remove support for the option altogether.

[MichaelChirico]

great point:

data.table/R/fread.R

Line 6 in 6cee825

showProgress=getOption("datatable.showProgress",interactive()), data.table=getOption("datatable.fread.datatable",TRUE),

There are indeed a fair number (O(50)) of downstream references to that option.

IMO datatable.show.progress seems more in line with how we tend to name options, e.g. we have datatable.show.indices already:

data.table/R/onLoad.R

Line 82 in 6cee825

"datatable.show.indices"="FALSE", # for print.data.table

But that horse is out of the barn already. I definitely don't think it's worth maintaining two separate options (datatable.showProgress, datatable.show.progress); are we OK with a minor inconsistency like:

`[.data.table` = function(..., show.progress = getOption("datatable.showProgress"), ...)
fread          = function(..., showProgress =  getOption("datatable.showProgress"), ...)
fwrite         = function(..., showProgress =  getOption("datatable.showProgress"), ...)

[joshhwuu]

This is definitely OK to me

I can write up a PR if everyone agrees

[jangorecki]

I don't think using same option for fread and group by is a good idea, maybe let's leave it default FALSE till someone request global option.

[tdhock]

I have a strong preference for keeping showProgress, for consistency with fread/fwrite

[phgrosjean]

Why not use {progressr} or the {progressr} logic ? It is possible, at least to set showProgress= (or whatever is chosen) dependent on a global option if it is not provided explicitly in the functions ? This would allow the end-user to decide if process is show or not, not the developer, see https://progressr.futureverse.org

[ben-schwen]

@phgrosjean I think the rationale behind this is that we do not want to introduce extra dependencies.

Adding a global options similar to verbose seems okayish in this case since we mainly affect verbosity and printing.

[MichaelChirico]

Sorry @phgrosjean , I'm not sure how what you're suggesting is different from what's currently available, could you elaborate?

Already we have the global option datatable.showProgress which can control the showProgress arguments to [, fwrite(), and fread().

[MichaelChirico]

@tdhock @jangorecki I'm not really sure how to proceed. I think we've found the first gap in our GOVERNANCE doc :)

Here's the timeline of where we're at AFAICT

• [Jul 8] @joshhwuu wrote Progress bar/indicator for "by" operations #6228 fulfilling progress indicator/bar for "by" operations #3060
• [Jul 16] @tdhock requested the PR include the argument showProgress= to [
• [Jul 29] @tdhock merged the PR: 4704c82. There was no activity from @jangorecki on the PR; the PR had achieved consensus as spelled out in GOVERNANCE.
• [Aug 5] In orthogonal PR Take a pass at condensing and tightening the NEWS #6348 by @MichaelChirico, @jangorecki expressed concern about mixing naming styles within the arguments of [
  • nomatch= and rollends= exhibit "minimal" style: no punctuation, no capitalization between words
  • allow.cartesian exhibits "R" style (maybe "dot notation" or "train case"): no punctuation, . between words
  • showProgress newly exhibits lowerCamelCase style
• [Aug 5] in response to this, @MichaelChirico suggested putting the argument name to a poll, which @jangorecki approved (:+1: the response).
• [Aug 13] @MichaelChirico posted a poll on fosstodon with a duration of 3 days, with 33% of 39 respondents requesting show.progress; showProgress was tied for 2nd with 23% of the vote (a difference of 4 votes).
• [Aug 16] @MichaelChirico opened this issue about renaming the argument to show.progress in response to the poll
• [Aug 16] @joshhwuu filed showProgress --> show.progress for "by" operations #6372 to fulfill the request
• [Aug 16] @tdhock expressed a strong preference to keep showProgress

My summary is that the change to showProgress= was made with ostensible consensus, but opposition was expressed by Committer @jangorecki after merging (less than a week later, and before CRAN release).

[jangorecki]

https://fosstodon.org/@michaelchirico/112956992965603990 :)

[tdhock]

don't let my strong preference block the change, I just want people to be aware of the inconsistency which I think would be suprising as a user

[MichaelChirico]

Thanks @tdhock. Indeed my own predilection is for showProgress=. However, users apparently disagree (by a small margin), per the poll.

Do you worry about the representativeness of the audience on fosstodon? (I could try re-running the poll on Twitter and/or LinkedIn?)

WDYT about using the shorter name progress= instead (at least then the naming styles don't specifically contradict each other across the functions, even though the name is different)?

[tdhock]

my concern is not about progress vs show.progress, it is that both would be different from the analog in fwrite/fread, showProgress.
I am not concerned about the representativeness of the audience on fosstodon vs twitter, and I'm not sure that either are aware of the two conflicting consistency goals (between arguments in [, and between functions [/fwrite/fread).
both kinds of inconsistency are undesirable.
the idea of maintaining two different argument/option names is too.

there is a third kind of consistency we need to consider here, consistency with base R (fread args vs read.table args, [.data.table vs [.data.frame).
this is hard!

maybe counting types of styles arguments could help? (we could use that to see what change would minimize some measure of inconsistency)

• "simple" means the argument is only one word/letter (no need for choice between camel/simple/R/etc)
• "abbrev" means one word (abbreviated).
• "acronym" means several words abbreviated using the first letter of each word.
• "minimal" means more than one word (possibly abbreviated), no space between.
• "dot" means . instead of space.
• "camel" means capitalize first letter of word instead of space.

my categorization of styles is below, feel free to edit/correct.

dt.fun	dt.arg.name	style	in.base
[.data.table	x	simple	TRUE
[.data.table	i	simple	TRUE
[.data.table	j	simple	TRUE
[.data.table	by	simple	FALSE
[.data.table	keyby	minimal	FALSE
[.data.table	with	simple	FALSE
[.data.table	nomatch	minimal	FALSE
[.data.table	mult	abbrev	FALSE
[.data.table	roll	simple	FALSE
[.data.table	rollends	minimal	FALSE
[.data.table	which	simple	FALSE
[.data.table	.SDcols	minimal	FALSE
[.data.table	verbose	simple	FALSE
[.data.table	allow.cartesian	dot	FALSE
[.data.table	drop	simple	TRUE
[.data.table	on	simple	FALSE
[.data.table	env	abbrev	FALSE
fread	input	simple	FALSE
fread	file	simple	TRUE
fread	text	simple	TRUE
fread	cmd	abbrev	FALSE
fread	sep	abbrev	TRUE
fread	sep2	abbrev	FALSE
fread	dec	abbrev	TRUE
fread	quote	simple	TRUE
fread	nrows	minimal	TRUE
fread	header	simple	TRUE
fread	na.strings	dot	TRUE
fread	stringsAsFactors	camel	TRUE
fread	verbose	simple	FALSE
fread	skip	simple	TRUE
fread	select	simple	FALSE
fread	drop	simple	FALSE
fread	colClasses	camel	TRUE
fread	integer64	minimal	FALSE
fread	col.names	dot	TRUE
fread	check.names	dot	TRUE
fread	encoding	simple	TRUE
fread	strip.white	dot	TRUE
fread	fill	simple	TRUE
fread	blank.lines.skip	dot	TRUE
fread	key	simple	FALSE
fread	index	simple	FALSE
fread	showProgress	camel	FALSE
fread	data.table	dot	FALSE
fread	nThread	camel	FALSE
fread	logical01	simple	FALSE
fread	keepLeadingZeros	camel	FALSE
fread	yaml	acronym	FALSE
fread	autostart	minimal	FALSE
fread	tmpdir	abbrev,minimal	FALSE
fread	tz	acronym	FALSE
fwrite	x	simple	TRUE
fwrite	file	simple	TRUE
fwrite	append	simple	TRUE
fwrite	quote	simple	TRUE
fwrite	sep	abbrev	TRUE
fwrite	sep2	abbrev	FALSE
fwrite	eol	acronym	TRUE
fwrite	na	acronym	TRUE
fwrite	dec	abbrev	TRUE
fwrite	row.names	dot	TRUE
fwrite	col.names	dot	TRUE
fwrite	qmethod	abbrev,minimal	TRUE
fwrite	logical01	minimal	FALSE
fwrite	logicalAsInt	camel	FALSE
fwrite	scipen	abbrev,minimal	FALSE
fwrite	dateTimeAs	camel	FALSE
fwrite	buffMB	abbrev,minimal	FALSE
fwrite	nThread	abbrev,camel	FALSE
fwrite	showProgress	camel	FALSE
fwrite	compress	simple	FALSE
fwrite	yaml	acronym	FALSE
fwrite	bom	acronym	FALSE
fwrite	verbose	simple	FALSE
fwrite	encoding	simple	FALSE

I made the table above using the R code below, then paste result to github, then typed in my judgment for style column.

library(data.table)
f <- function(dt.fun, base.fun)data.table(dt.fun, base.fun)
knitr::kable(rbind(
  f('[.data.table','[.data.frame'),
  f('fread','read.table'),
  f('fwrite','write.table')
)[, {
  dt.arg.name <- names(formals(getFromNamespace(dt.fun,"data.table")));
  .(dt.arg.name, style="", in.base=dt.arg.name %in% names(formals(get(base.fun))))
}, by=dt.fun])