Issues in translatable messages in R code

[aitap]

Fragmented sentences

• %s is elements of fun.list or ...arguments to measure or maybe measured columns, untranslated, coming from the group.desc argument, which is a documented ("for internal use") interface. Another %s is number of capture groups in pattern or max number of items after splitting column names, also untranslated.
  

  data.table/R/fmelt.R

  Line 91 in 745160d

  stopf("in measurev, %s must be named, problems: %s", group.desc, brackify(prob.i))

  
  

  data.table/R/fmelt.R

  Line 97 in 745160d

  stopf("%s should be uniquely named, problems: %s", err.what, brackify(names(bad.counts)))

  
  

  data.table/R/fmelt.R

  Line 102 in 745160d

  stopf("number of %s =%d must be same as %s =%d", group.desc, length(fun.list), type, N)

• %1$s is keys or indices, untranslated.
  

  data.table/R/setops.R

  Lines 160 to 177 in 745160d

  	return(gettextf(
  	"Datasets have different %s. 'target': %s. 'current': %s.",
  	"keys",
  	if(length(k1)) brackify(k1) else gettextf("has no key"),
  	if(length(k2)) brackify(k2) else gettextf("has no key")
  	))
  	}
  	# check index
  	i1 = indices(target)
  	i2 = indices(current)
  	if (!identical(i1, i2)) {
  	return(gettextf(
  	"Datasets have different %s. 'target': %s. 'current': %s.",
  	"indices",
  	if(length(i1)) brackify(i1) else gettextf("has no index"),
  	if(length(i2)) brackify(i2) else gettextf("has no index")
  	))
  	}

• %3$s is either error or warning, untranslated. That's relatively minor and could be worked around in translation (condition of class %s):
  

  data.table/R/fread.R

  Line 314 in 745160d

  warningf("Column '%s' was requested to be '%s' but fread encountered the following %s:\n\t%s\nso the column has been left as type '%s'", names(ans)[j], new_class, if (inherits(e, "error")) "error" else "warning", e$message, typeof(v))

Minor issues with correctness

• This should have said POSIXct, not just POSIX (which could be read as POSIXt or POSIXlt, and we don't like POSIXlt):
  

  data.table/R/between.R

  Line 13 in 745160d

  "'between' function the 'x' argument is a POSIX class while '%s' was not, coercion to POSIX failed with: %s", 'lower', e$message))

  
  

  data.table/R/between.R

  Line 15 in 745160d

  "'between' function the 'x' argument is a POSIX class while '%s' was not, coercion to POSIX failed with: %s", 'upper', e$message))

• Missing ] at the end:
  

  data.table/R/data.table.R

  Line 295 in 745160d

  stopf("You have wrapped := with {} which is ok but then := must be the only thing inside {}. You have something else inside {} as well. Consider placing the {} on the RHS of := instead; e.g. DT[,someCol:={tmpVar1<-...;tmpVar2<-...;tmpVar1*tmpVar2}")

• The last } needs to be replaced with ]
  

  data.table/R/data.table.R

  Line 412 in 745160d

  "'%s' is not found in calling scope, but it is a column of type %s. If you wish to select rows where that column contains TRUE, or perhaps that column contains row numbers of itself to select, try DT[(col)], DT[DT$col], or DT[col==TRUE} is particularly clear and is optimized",

• PR17478 is now FIXED CLOSED:
  

  data.table/R/onLoad.R

  Line 27 in 745160d

  stopf("The data_table.%s version (%s) does not match the package (%s). Please close all R sessions to release the old %s and reinstall data.table in a fresh R session. The root cause is that R's package installer can in some unconfirmed circumstances leave a package in a state that is apparently functional but where new R code is calling old C code silently: https://bugs.r-project.org/bugzilla/show_bug.cgi?id=17478. Once a package is in this mismatch state it may produce wrong results silently until you next upgrade the package. Please help by adding precise circumstances to 17478 to move the status to confirmed. This mismatch between R and C code can happen with any package not just data.table. It is just that data.table has added this check.", dll, dllV, RV, toupper(dll))

Nitpicking

• In R ≥ 4.2.0 there's the Sys.setLanguage function which (tries to) set the gettext language during runtime, maybe recommend it here?
  

  data.table/R/onAttach.R

  Line 28 in 745160d

  packageStartupMessagef("**********\nRunning data.table in English; package support is available in English only. When searching for online help, be sure to also check for the English error message. This can be obtained by looking at the po/R-<locale>.po and po/<locale>.po files in the package source, where the native language and English error messages can be found side-by-side\n**********")

• Looks like the code accepts a single data.table/data.frame, maybe remove the s?
  

  data.table/R/duplicated.R

  Line 105 in 745160d

  stopf("x must be an atomic vector or data.frames/data.tables")

• s/both libraries/both packages/
  

  data.table/R/fmelt.R

  Line 17 in 745160d

  warningf("The %1$s generic in data.table has been passed a %2$s and will attempt to redirect to the relevant reshape2 method; please note that reshape2 is superseded and is no longer actively developed, and this redirection is now deprecated. To continue using melt methods from reshape2 while both libraries are attached, e.g. melt.list, you can prepend the namespace, i.e. reshape2::%1$s(%3$s). In the next version, this warning will become an error.", "melt", class1(data), data_name)

• Maybe shouldn't be translated
  

  data.table/R/setops.R

  Line 9 in 745160d

  if (length(by.x) != length(by.y)) stopf("length(by.x) != length(by.y)")

• Needs # nontranslate because it's from a different domain
  

  data.table/R/test.data.table.R

  Line 118 in 745160d

  foreign = gettext("object '%s' not found", domain="R") != "object '%s' not found"

• Perhaps ...vector **of** length %d?
  

  data.table/R/data.table.R

  Line 1043 in 745160d

  if (length(.SDcols)!=length(x)) stopf(".SDcols is a logical vector length %d but there are %d columns", length(.SDcols), length(x))

[rikivillalba]

• In fmelt
  

  data.table/R/fmelt.R

  Line 100 in 4ffe958

  err.args.groups = function(type, N){

err.args.groups = function(type, N){
  if (N != length(fun.list)) {
    stopf("number of %s =%d must be same as %s =%d", group.desc, length(fun.list), type, N)
  }
}

type is an argument that is not translated, i.e. in line 120
err.args.groups("number of capture groups in pattern", ncol(start))

data.table/R/fmelt.R

Line 120 in 4ffe958

err.args.groups("number of capture groups in pattern", ncol(start))

[MichaelChirico]

@tdhock can you help me understand measurev() here? It has an argument group.desc= with a default value "elements of fun.list". But I only ever see measurev() called in one place, with a different value:

data.table/R/fmelt.R

Line 70 in 6abbaaf

list(group.desc="... arguments to measure"))

Can we just drop the group.desc argument, since measurev() is unexported? Also, remind me why it is documented given that it's unexported?

[tdhock]

since measure is exported, we should export measurev too. both are documented because they are meant to be used as the measure.vars argument to melt.
when I wrote measure/measurev, I was not aware of the issues created by breaking strings apart when translating messages.
so we should probably get rid of the group.desc argument, I will look at #6534

[MichaelChirico]

OK, filed #6535.

[MichaelChirico]

I see now in the tests that measurev() is called with the default group.desc=, I guess some NSE applies there.

[tdhock]

the user could call either measure() or measurev() inside measure.vars.
measure() calls measurev() but we want specific error messages to help the user:

> melt(data.table(iris)[1], measure.vars=measure(part, part))
Error in err.names.unique(group.desc, names(fun.list)) : 
  ... arguments to measure should be uniquely named, problems: [part]
> melt(data.table(iris)[1], measure.vars=measurev(list(part=NULL, part=NULL)))
Error in err.names.unique(group.desc, names(fun.list)) : 
  elements of fun.list should be uniquely named, problems: [part]

[MichaelChirico]

Yea, I agree it's not perfect, but it certainly looks actionable to me because part is clearly duplicated.

[rikivillalba]

In deve.R I think we can merge
https://github.com/Rdatatable/data.table/blob/master/R/devel.R#L40:L44

    cat(sprintf("R %s package %s %s (%s)\n",
                pkg,
                c("is up-to-date at","has been updated to")[upg+1L],
                unname(read.dcf(system.file("DESCRIPTION", package=pkg, lib.loc=lib, mustWork=TRUE), fields=field)[, field]),
                utils::packageVersion(pkg, lib.loc=lib)))

into

    pkg_rev = unname(read.dcf(system.file("DESCRIPTION", package=pkg, lib.loc=lib, mustWork=TRUE), fields=field)[, field])
    if(upg) {
      catf("R %s package %s has been updated to (%s)\n",  pkg, pkg_rev, utils::packageVersion(pkg, lib.loc=lib))
    } else {
      catf("R %s package %s is up-to-date at (%s)\n",  pkg, pkg_rev, utils::packageVersion(pkg, lib.loc=lib))
    }

I think we can use catf despite the previous line unloads the package.

[aitap]

This is tricky code. Objects that live in package namespaces are "lazy-loaded": the namespace environment initially contains "promise" objects that, when first accessed, quickly seek to pre-remembered offsets in R/data.table.rdb and uncompress whatever they find there. This function unloads the namespace and installs a new package, replacing the *.rdb file with a different one, where the offsets have changed. If the catf function is still a promise at this point, it will at least cause a warning (internal error -3 in R_decompress1), maybe fail to evaluate (lazy-load DB is corrupt):

Downloaded package sources are in
        ‘/tmp/Rtmp3GzX83/downloaded_packages’
R data.table package 0c957c70d019ebf36c44c988d5b2a4def9c05c92 has been updated to (1.16.99)
Warning:
In get(method, envir = home) : internal error -3 in R_decompress1

It may be better to limit the "last on the way out" code to base-R functions only:

    pkg_rev = unname(read.dcf(system.file("DESCRIPTION", package=pkg, lib.loc=lib, mustWork=TRUE), fields=field)[, field])
    cat(sprintf(
      if (upg) gettext(
        "R %s package %s has been updated to (%s)\n", domain = "R-data.table"
      ) else gettext(
        "R %s package %s is up-to-date at (%s)\n", domain = "R-data.table"
      ),
      pkg, pkg_rev, utils::packageVersion(pkg, lib.loc=lib)
    ))

[MichaelChirico]

Closed between #6534 and #6536