210 depracate datanames in teal.data::get_code

[m7pr]

Part of

• [Feature Request]: Implement names argument in get_code to subset code for specific objects. teal.code#210

Companion to

• 210 substitute teal.data::get_code with teal.code::get_code teal#1388
• 210 bring get_code_dependency teal.code#214

Description

Deprecates the usage of datanames parameter in teal.data::get_code(datanames) in favour of teal.code::get_code(names).

VBUMP package version locally to something greater than 0.6.1 during testing.

library(teal.data)
#> Loading required package: teal.code
tdata1 <- within(teal_data(), {a <- 1; b <- 2})
get_code(tdata1)
#> [1] "a <- 1\nb <- 2"
get_code(tdata1, datanames = "a")
#> Warning: The `datanames` argument of `get_code()` is deprecated as of teal.data 0.6.1.
#> ℹ Please use the `names` argument of `teal.code::get_code()` instead.
#> ℹ The deprecated feature was likely used in the teal.data package.
#>   Please report the issue at
#>   <https://github.com/insightsengineering/teal.data/issues>.
#> This warning is displayed once every 8 hours.
#> Call `lifecycle::last_lifecycle_warnings()` to see where this warning was
#> generated.
#> [1] "a <- 1"

^{Created on 2024-10-17 with reprex v2.1.1}

[m7pr]

I think it should be OK to just delete this method and make a case in teal.code::get_code that if someone uses datanames
parameter than this is treated as names parameter and a warning is thrown that you should use names. If this method in teal.data gets removed, user automatically will use teal.code::get_code that will understand names and datanames.

What do you think @gogonzo ?

[gogonzo]

I think it should be OK to just delete this method and make a case in teal.code::get_code that if someone uses datanames parameter than this is treated as names parameter and a warning is thrown that you should use names. If this method in teal.data gets removed, user automatically will use teal.code::get_code that will understand names and datanames.

What do you think @gogonzo ?

I'm not sure if we can just remove a method. Some people might use teal.data::get_code() and thus they might be surprised and lost (because there will be no info about a move to teal.code).

[m7pr]

@gogonzo valid point, if someone prefixes get_code with the name of the package and uses teal.data then they might be lost. Let's don't just delete then

[m7pr]

Updated the opening comment and made this ready for the review!

[github-actions]

Code Coverage Summary

Filename                     Stmts    Miss  Cover    Missing
-------------------------  -------  ------  -------  --------------------
R/cdisc_data.R                   1       0  100.00%
R/deprecated.R                  57      57  0.00%    19-344
R/dummy_function.R               2       2  0.00%    14-15
R/formatters_var_labels.R       61       0  100.00%
R/join_key.R                    38       0  100.00%
R/join_keys-c.R                 12       0  100.00%
R/join_keys-extract.R          128       0  100.00%
R/join_keys-names.R             15       0  100.00%
R/join_keys-parents.R           30       0  100.00%
R/join_keys-print.R             45       0  100.00%
R/join_keys-utils.R             73       3  95.89%   35-38
R/join_keys.R                   22       0  100.00%
R/teal_data-class.R             26       1  96.15%   69
R/teal_data-datanames.R         20       0  100.00%
R/teal_data-get_code.R          13       8  38.46%   112-118, 122
R/teal_data-show.R               4       4  0.00%    14-19
R/teal_data.R                   30      16  46.67%   33, 36-42, 52-58, 61
R/testhat-helpers.R             26       0  100.00%
R/topological_sort.R            32       0  100.00%
R/verify.R                      42      11  73.81%   65, 95-99, 102-106
TOTAL                          677     102  84.93%

Diff against main

Filename                  Stmts    Miss  Cover
----------------------  -------  ------  -------
R/teal_data-get_code.R       -1      +8  -61.54%
TOTAL                        -1      +8  -4.15%

Results for commit: ff7cc1d

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

[github-actions]

Unit Tests Summary

  1 files   13 suites   1s ⏱️
162 tests 162 ✅ 0 💤 0 ❌
219 runs  219 ✅ 0 💤 0 ❌

Results for commit ff7cc1d.

♻️ This comment has been updated with latest results.

[github-actions]

Unit Test Performance Difference

Test Suite	$Status$	Time on main	$±Time$	$±Tests$	$±Skipped$	$±Failures$	$±Errors$
get_code	💀	$0.90$	$-0.90$	$-64$	$-2$	$0$	$0$

Additional test case details

Test Suite	$Status$	Time on main	$±Time$	Test Case
get_code	💀	$0.02$	$-0.02$	linksto_gets_extracted_if_it_s_a_side_effect_on_a_dependent_object_even_of_a_dependent_object
get_code	💀	$0.01$	$-0.01$	_linksto_makes_a_line_being_returned_for_an_affected_binding
get_code	💀	$0.01$	$-0.01$	_linksto_returns_the_line_for_an_affected_binding_even_if_the_object_did_not_exist_in_the_same_iteration_of_eval_code
get_code	💀	$0.03$	$-0.03$	code_can_be_retrieved_with_get_code
get_code	💀	$0.02$	$-0.02$	data_call_is_returned_when_data_name_is_provided_as_a_character
get_code	💀	$0.02$	$-0.02$	data_call_is_returned_when_data_name_is_provided_as_is
get_code	💀	$0.01$	$-0.01$	detects_cooccurrence_properly_even_if_all_objects_are_on_lhs
get_code	💀	$0.01$	$-0.01$	detects_every_assign_calls_even_if_not_evaluated_if_there_is_only_one_assignment_in_this_line
get_code	💀	$0.01$	$-0.01$	detects_function_usage_of_the_assignment_operator
get_code	💀	$0.01$	$-0.01$	detects_occurrence_of_a_function_definition_when_a_formal_is_named_the_same_as_a_function
get_code	💀	$0.02$	$-0.02$	detects_occurrence_of_a_function_definition_with_a_linksto_usage
get_code	💀	$0.02$	$-0.02$	detects_occurrence_of_the_function_object
get_code	💀	$0.01$	$-0.01$	does_not_break_if_code_is_separated_by_
get_code	💀	$0.01$	$-0.01$	does_not_break_if_code_uses_quote_
get_code	💀	$0.02$	$-0.02$	does_not_break_if_object_is_used_in_a_function_on_lhs
get_code	💀	$0.01$	$-0.01$	does_not_break_if_object_is_used_in_a_function_on_lhs_and_influencers_are_both_on_lhs_and_rhs
get_code	💀	$0.03$	$-0.03$	does_not_fall_into_a_loop
get_code	💀	$0.00$	$-0.00$	does_not_ignore_occurrence_in_function_body_if_object_exsits_in_env
get_code	💀	$0.01$	$-0.01$	extracts_code_of_a_parent_binding_but_only_those_evaluated_before_coocurence
get_code	💀	$0.05$	$-0.05$	extracts_the_code_for_assign_where_x_is_a_literal_string
get_code	💀	$0.00$	$-0.00$	extracts_the_code_for_assign_where_x_is_variable
get_code	💀	$0.01$	$-0.01$	extracts_the_code_of_a_binding_from_character_vector_containing_simple_code
get_code	💀	$0.01$	$-0.01$	extracts_the_code_of_a_parent_binding_if_used_as_an_arg_in_a_function_call
get_code	💀	$0.01$	$-0.01$	extracts_the_code_when_using_
get_code	💀	$0.01$	$-0.01$	extracts_the_code_without_downstream_usage
get_code	💀	$0.01$	$-0.01$	get_code_does_not_break_if_linksto_is_put_in_the_last_line
get_code	💀	$0.04$	$-0.04$	handles_empty_code_slot
get_code	💀	$0.01$	$-0.01$	handles_the_code_included_in_curly_brackets
get_code	💀	$0.01$	$-0.01$	handles_the_code_of_length_1_when_at_least_one_is_enclosed_in_curly_brackets
get_code	💀	$0.01$	$-0.01$	handles_the_code_without_symbols_on_rhs
get_code	💀	$0.02$	$-0.02$	ignores_occurrence_in_a_function_definition
get_code	💀	$0.04$	$-0.04$	ignores_occurrence_in_a_function_definition_if_there_is_multiple_function_definitions
get_code	💀	$0.02$	$-0.02$	ignores_occurrence_in_a_function_definition_in_lapply
get_code	💀	$0.03$	$-0.03$	ignores_occurrence_in_a_function_definition_that_has_function_in_it
get_code	💀	$0.05$	$-0.05$	ignores_occurrence_in_function_definition_without_curly_brackets
get_code	💀	$0.01$	$-0.01$	library_and_require_are_always_returned
get_code	💀	$0.02$	$-0.02$	lines_affecting_parent_evaluated_after_co_occurrence_are_not_included_in_output_when_using_linksto
get_code	💀	$0.01$	$-0.01$	returns_result_of_length_1_for_non_empty_input
get_code	💀	$0.01$	$-0.01$	starting_with_underscore_is_detected_in_code_dependency
get_code	💀	$0.11$	$-0.11$	understands_usage_and_do_not_treat_rhs_of_as_objects_only_lhs_
get_code	💀	$0.01$	$-0.01$	warns_if_binding_doesn_t_exist_in_code
get_code	💀	$0.01$	$-0.01$	with_non_native_pipe_is_detected_code_dependency
get_code	💀	$0.01$	$-0.01$	with_non_native_pipe_used_as_function_is_detected_code_dependency
get_code	💀	$0.02$	$-0.02$	with_space_character_is_detected_in_code_dependency
get_code	💀	$0.01$	$-0.01$	without_special_characters_is_cleaned_and_detected_in_code_dependency
get_code	💀	$0.04$	$-0.04$	works_for_assign_detection_no_matter_how_many_parametrers_were_provided_in_assignq_
get_code	💀	$0.01$	$-0.01$	works_for_datanames_of_length_1

Results for commit 43f8eca

♻️ This comment has been updated with latest results.

[gogonzo]

There is no point of keeping the old code. get_code should just use teal.code::get_code.

# method for teal_data
function(object, deparse, datanames = NULL, names = datanames, ...) {
  <deprecate>
  teal.code::get_code(object = object,  deparse = deparse, names = names) 
}

Current implementation also assumes that get_code method for teal_data won't be deprecated (that it will stay) because it warns-deprecate only when datanames is specified.

[m7pr]

@gogonzo totally! This is way easier and cleaner. Will improve! Thanks

[m7pr]

I see some local errors, that are also on CI/GitHub Actions

> library(teal.data)
> tdata1 <- within(teal_data(), {a <- 1; b <- 2})
> get_code(tdata1)
Error in grDevices::pdf(nullfile()) : too many open devices
Called from: grDevices::pdf(nullfile())

grDevices::pdf is called in the generic in teal.code

[m7pr]

Thanks @pawelru for pointing out the deprecated and is_present. Very useful set of tools

I incorporated those functions in this commit 36e0a33

[gogonzo]

👍

this is covered now