220 detect default side effects #230
Conversation
|
Code Coverage SummaryDiff against mainResults for commit: 2e19f52 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Unit Test Performance DifferenceAdditional test case details
Results for commit 0134fb8 ♻️ This comment has been updated with latest results. |
|
Objection to |
|
Yeah, had similar thoughts. I think the most flexible will be an exposure
of a parameter vector in get_code that would take a list of names that are
always returned. This could default to 'library' and be changed by a user.
|
|
Let's try to think of something else before we add arguments for everything. |
|
It's always the matter of what are the cases and needs of users. I think in
this specific situation there are a lot of unforeseen situations in which a
user would assume some specific function calls should be returned because
they are needed. I think libraries and require are fine for a good start.
Then maybe `options` and `set.seed`. But what else? There might be a lot
and possibly each user has different needs. That's why I am proposing a
parameter. This function covers a very complicated problem so I would
imagine we can have 4 or 5 parameters. Simple `lm` function for linear
regression has 12 parameters.
I don't think we're gonna cover every scenario, even if we think of
something else. I think introduction of parameters is fine if you see such
a situation where the usage has a broad scope.
…On Mon, Dec 18, 2023 at 3:03 PM Aleksander Chlebowski < ***@***.***> wrote:
Let's try to think of something else before we add arguments for
everything.
—
Reply to this email directly, view it on GitHub
<#230 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A74AIELHJ6N4PKGXYTCH3QTYKBEMLAVCNFSM6AAAAABAZPLBG6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRQGU4DMOBQG4>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
If a person has to manage what function calls are "automatically" included, they might as well manually tag them. A reasonable exception is the search path, that one has to be reproduced. |
|
Hey, so far I'm only automatically extracting |
Yes but in general I'm looking to resolve |
|
ALrighty, let's move there for the conversation about library() simplification. And for this feature, do you think we're good to approve and merge? |
Co-authored-by: Dawid Kałędkowski <6959016+gogonzo@users.noreply.github.com> Signed-off-by: Marcin <133694481+m7pr@users.noreply.github.com>
|
Thanks for the review. Badge was added and test were simplified |
**Merge after** #230 Closes #233 Added one more text for future improvement, where we are not able to handle `x` parameter passed to `assign` function as `variable`. Such specs for `assign` are handled ```r code <- c( "a <- 1", "assign('b', 5)", "assign(value = 7, x = 'c')", "b <- b + 2", "c <- b" ) ``` and below is not (yet!) ```r code <- c( "z <- 'a'", "assign(x = z, 5)", "b <- a" ) ``` --------- Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Close #220
We introduced a list of functions that are treated as default side effects having impact on all objects. Those will be always returned with
get_code. Currently the list contains oflibrary,requireanddata. I wonder if this should be a parameter toget_codeso that this list can be extended and adjusted by future users?