Update Documentation for env Parameter Usage*

[Nj221102]

closes #6323

Description:

This PR updates the documentation for the env parameter in the data.table package to include new functionality regarding function injection. Initially, the documentation update was focused solely on using function names as strings (e.g., "sum"), as requested in issue #6323. However, following the merge of PR #6026 which allows the direct use of function objects (e.g., sum). This PR updates the documentation to reflect both approaches.

Changes:

1. Documentation Update:

   • The documentation now includes both methods for injecting functions using the env parameter:
     • Function Names as Strings: Using function names as strings (e.g., "sum").
     • Function Objects Directly: Passing function objects directly (e.g., sum).

2. Example Block Addition:

   • Added examples demonstrating both approaches for function injection.

[TysonStanley]

LGTM

[tdhock]

I removed the DT definition since there are already other DT definitions that we can use for this example.
I noticed this would add the first example use of env arg to ?data.table man page.
@jangorecki is there some reason why there were no env examples before?

[MichaelChirico]

Thanks for the PR! Actually, for addressing #6323 I had in mind extending the programming vignette, rather than ?data.table.

Although as Toby points out, it looks like this is the first example using env= on that page, so it's good to keep the new example.

[Nj221102]

Thanks for the PR! Actually, for addressing #6323 I had in mind extending the programming vignette, rather than ?data.table.

Although as Toby points out, it looks like this is the first example using env= on that page, so it's good to keep the new example.

sure, i m updating the programming vignette now.

[jangorecki]

Direct usage of sum rather than "sum" has to be avoided. Please investigate yourself. Take some big R function, and use it that way, verbose should display what's wrong. substitute2 may be even easier to show the problem.

I can imagine it could even potentially fail on trying to build big expression in j.

The only reason to use function name as a symbol rather than string is when we want to inject function body into expression. I see no reasonable use case for that.

Definitly worth to add some examples to manual as well. Now it was only redirecting to vignette and substitute2 manual.

[tdhock]

by the way Jan it is really super convenient how you can substitute names via substitute2 / env, thanks for implementing that.

[tdhock]

@Nj221102 please also add discussion/example to programming vignette, maybe under ### `get` ?

[jangorecki]

by the way Jan it is really super convenient how you can substitute names via substitute2 / env, thanks for implementing that.

Yes that was the "new" thing vs. substitute or tidy way of meta programming. Afair that was the only part that required to be coded in C.
https://github.com/Rdatatable/data.table/blob/d00f292596d82fd21aac59edc34a9f8b388450f4/src/programming.c

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (master@1b3998e). Learn more about missing BASE report.
Report is 9 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##             master    #6360   +/-   ##
=========================================
  Coverage          ?   98.34%           
=========================================
  Files             ?       81           
  Lines             ?    14927           
  Branches          ?        0           
=========================================
  Hits              ?    14680           
  Misses            ?      247           
  Partials          ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[MichaelChirico]

Direct usage of sum rather than "sum" has to be avoided. Please investigate yourself. Take some big R function, and use it that way, verbose should display what's wrong. substitute2 may be even easier to show the problem.

I can imagine it could even potentially fail on trying to build big expression in j.

The only reason to use function name as a symbol rather than string is when we want to inject function body into expression. I see no reasonable use case for that.

Definitly worth to add some examples to manual as well. Now it was only redirecting to vignette and substitute2 manual.

Elaborated in the original issue (#6323) on why I continue to advocate for direct usage of sum.

For now, I've opted to just put a minimal mention of that to resolve this issue. Beyond that it's something we can continue to take into consideration as we observe user reports/our own usage.

[jangorecki]

I think there was some misconception at the start of this PR.

Initially, the documentation update was focused solely on using function names as strings (e.g., "sum"), as requested in issue #6323. However, following the merge of PR #6026 which allows the direct use of function objects (e.g., sum).

#6026 says

Note that it's much better to pass functions like f="sum" instead.

For a good reason, because 99.5% of the time we will not want to substitute data.table expression using body of the function.

sum will work but it won't be calling sum anymore, it will be pasting in body of the sum function and evaluating that body.

Therefore I don't think it is reasonable to document that as a way to do it.

I will submit an edit to this PR in a moment which should make everything more clear.

[jangorecki]

Mention of do.call is very on point here

f = function() sys.call()
do.call("f",list())
#f()
do.call(f,list())
#(function() sys.call())()

and imagine your f function is something big... [.data.table has 1850 lines of code

[jangorecki]

As written now, I hope it will prevent any misconception on that matter in future.
I moved it to new section as it didn't fit well to the one used previously.
Please review.

[MichaelChirico]

this one in particular feels quite unnatural to me -- almost never do we want to separate the namespace from its export. I can't think of any use case for that. and it gets us even further from one of my main points about do.call() -- using symbols makes the dependency structure of the code much clearer.

let's drop this example. I think the above section is a nice addition -- we can revisit the point about namespace-qualified inputs as we gather more organic use cases in the wild.

[jangorecki]

It can as well be base::fun(col), env=list(fun="sqrt")
And also nsfun(col), env=call("::", as.name("base"), as.name("sqrt"))

[jangorecki]

Or even parse(text="base::sqrt") which I would prefer not to mention in our docs

[jangorecki]

using symbols makes the dependency structure of the code much clearer.

The problem is that it changes the code.

Symbols should be used, and should be preferred but then why to use them in env? They don't need substation

[MichaelChirico]

nsfun(col), env=call("::", as.name("base"), as.name("sqrt"))
Or even parse(text="base::sqrt") which I would prefer not to mention in our docs

these days str2lang("base::sqrt") would be better and is sort of "accepted"; anyway, now I think you're hitting on something more interesting: fun(col); env = list(fun = quote(base::sqrt)) is pretty natural and I think solves all of my concerns.

[aitap]

Is there a situation when an i/j/by-expression would need to be evaluated in a completely different context from where it had been written? Would a user normally want to access a function or a piece of data from an environment that's neither the data.table itself nor the frame of the caller of [? Similar to what groupingsets does, hiding its own frame from [ but making the caller's frame available, except with the query itself, not the overall call to [?

[jangorecki]

DT[, fun(col), env = list(fun = quote(base::sqrt))]

It is then more correct to write

DT[, base::sqrt(col)]

As there is nothing to substitute. No advantage of using env.

[jangorecki]

@aitap don't really know now without a nice example showing the issue, env is force evaluated, i/j/by after substation should in theory behave as written by hand, so scoping should not make any difference than [ written by hand

[MichaelChirico]

It is then more correct to write

Right, in the simple case... I am still fairly certain this is needed for more generality. I will let this percolate for some time. In the meantime, let's just drop the ugly ns::fun example which I don't think is useful & we can merge this?