Skip to content

Export splv_extra and value_expr accessors#1098

Merged
Melkiades merged 5 commits into
insightsengineering:mainfrom
munoztd0:export_symbols_junco
Jul 1, 2026
Merged

Export splv_extra and value_expr accessors#1098
Melkiades merged 5 commits into
insightsengineering:mainfrom
munoztd0:export_symbols_junco

Conversation

@munoztd0

Copy link
Copy Markdown
Contributor

Exports two previously internal accessors needed by downstream package (junco).

splv_extra / splv_extra<- : get/set child-specific extra arguments on a SplitValue object

value_expr : retrieve the subsetting expression from a SplitValue / ValueWrapper object

see -> johnsonandjohnson/junco#318

@github-actions

github-actions Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

✅ All contributors have signed the CLA
Posted by the CLA Assistant Lite bot.

@munoztd0

Copy link
Copy Markdown
Contributor Author

@gmbecker

@munoztd0

Copy link
Copy Markdown
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@Melkiades Melkiades left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

examples fail check (SplitValue() not exported) and docs need re-document (int_methods.Rd still has the value_expr/splv_extra aliases, now duplicated). minor typo in NEWS

Comment thread R/tree_accessors.R
Comment thread R/tree_accessors.R
Comment thread NEWS.md Outdated
@munoztd0

munoztd0 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

FYI, document() is quite outdated:

image

@munoztd0

munoztd0 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

examples fail check (SplitValue() not exported) and docs need re-document (int_methods.Rd still has the value_expr/splv_extra aliases, now duplicated). minor typo in NEWS

Good catch, updated !

@Melkiades Melkiades left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm! Thanks @munoztd0

@Melkiades Melkiades left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you fix the CICD? Thanks

@munoztd0

munoztd0 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

Could you fix the CICD? Thanks

Ok I am fixing the lintr issues %>% to |> and the WORDLIST even if that's not my changes

image

but what about the links ?

image

Comment thread tests/testthat/test-accessors.R
@Melkiades
Melkiades enabled auto-merge (squash) July 1, 2026 13:04

@Melkiades Melkiades left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm! Thanks @munoztd0 for the fast reaction times : )

@Melkiades
Melkiades merged commit 7f4838e into insightsengineering:main Jul 1, 2026
28 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 1, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants