join_key can set default parent

[averissimo]

Pull Request

Fixes #203

Changes description / TODO while wip

• Add parameter on join_key() and [<-.join_keys that determines which dataset is parent
• Protects against cyclical parents when creating or merging join_keys
• Tests
  • Corrects existing tests to include new logic
  • Re-write / add tests to account for usage
• Update documentation
  • Man pages
  • Update join_key vignette

🗨️ Discussion

• Q: should jk[[i]][[j]] <- key define a parent relationship?
  • consider that jk[[i]] <- list(j = key, m = key2) would have to set parents for j and m
    • might also have to add parent parameter.
  • My intuition is that [[ should be left undirected

[github-actions]

Code Coverage Summary

Filename                         Stmts    Miss  Cover    Missing
-----------------------------  -------  ------  -------  -------------------
R/cdisc_data.R                       1       1  0.00%    37
R/default_cdisc_join_keys.R         11      11  0.00%    17-35
R/deprecated.R                      69      69  0.00%    16-350
R/dummy_function.R                   5       5  0.00%    16-23
R/formatters_var_labels.R           47      47  0.00%    28-149
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%   33-36
R/join_keys.R                       21       0  100.00%
R/teal_data-class.R                 25       1  96.00%   70
R/teal_data-datanames.R             10       0  100.00%
R/teal_data-get_code.R              15       1  93.33%   55
R/teal_data-show.R                   4       4  0.00%    13-18
R/teal_data.R                       22       9  59.09%   32, 41-47, 50
R/testhat-helpers.R                 26       0  100.00%
R/topological_sort.R                32       0  100.00%
R/utils-get_code_dependency.R      104       1  99.04%   43
R/utils.R                           14       0  100.00%
R/verify.R                          42      11  73.81%   68, 98-102, 105-109
R/zzz.R                             10      10  0.00%    4-16
TOTAL                              799     173  78.35%

Diff against main

Filename                 Stmts    Miss  Cover
---------------------  -------  ------  --------
R/join_key.R                +7       0  +100.00%
R/join_keys-c.R            -14       0  +100.00%
R/join_keys-extract.R       +6       0  +100.00%
R/join_keys-parents.R       +2      -1  +3.57%
R/join_keys-print.R         -1       0  +100.00%
R/join_keys-utils.R        -14      +1  -1.81%
TOTAL                      -14       0  -0.37%

Results for commit: 86529be

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

[github-actions]

Unit Tests Summary

    1 files    14 suites   1s ⏱️
168 tests 166 ✔️ 2 💤 0 ❌
247 runs  245 ✔️ 2 💤 0 ❌

Results for commit 86529be.

♻️ This comment has been updated with latest results.

[github-actions]

Unit Test Performance Difference

Additional test case details

Test Suite	$Status$	Time on main	$±Time$	Test Case
join_keys-c	💀	$0.01$	$-0.01$	c.join_key_set_throws_on_conflicting_join_keys_set_objects
join_keys-c	👶		$+0.01$	c.join_keys_throws_error_when_merge_produces_acyclical_graph
join_keys-c	💀	$0.02$	$-0.02$	c.join_keys_throws_on_conflicting_join_keys_set_objects
join_keys-extract	💀	$0.01$	$-0.01$	_.join_keys_adds_symmetrical_change_to_the_foreign_dataset
join_keys-extract	👶		$+0.00$	_.join_keys_adds_symmetrical_change_without_parents_to_the_foreign_dataset
join_keys-extract	💀	$0.01$	$-0.01$	_.join_keys_removes_keys_with_NULL_and_applies_summetrical_changes
join_keys-extract	👶		$+0.01$	_.join_keys_removes_keys_with_NULL_and_applies_symmetrical_changes
join_keys-extract	👶		$+0.01$	join_keys_i_j_removes_keys_with_NULL
join_keys-parents	👶		$+0.00$	parents_will_return_empty_list_when_attribute_does_not_exist
join_keys	👶		$+0.01$	join_keys_accepts_duplicated_join_key_undirected_
join_keys	👶		$+0.01$	join_keys_cannot_create_acyclical_graph
join_keys	💀	$0.03$	$-0.03$	join_keys_fails_when_provided_foreign_key_pairs_have_incompatible_values

Results for commit 3270664

♻️ This comment has been updated with latest results.

[gogonzo]

parent argument is confusing. Default values differ between two functions and the possibility of having a parent as dataset_1 or dataset_2 is too "freely". I would prefer to limit dataset_1 to be a parent only. So that

join_key("parent", "child", c(id_in_parent = id_in_child), directed = TRUE) # default

join_key("A", "B", c(id_in_A = id_in_B), directed = FALSE)

Comments addressed

[kartikeyakirar]

@averissimo PR looks great. I have re-reviewed it and tested it with c(...) and join_keys(...). Additionally, I found three unused variables (by running the linter), which I have removed here 7947878

[kartikeyakirar]

👍