Skip to content

Comments

add show_labels function#63

Open
toddyoder-mrg wants to merge 6 commits intometrumresearchgroup:mainfrom
toddyoder-mrg:feature/add-show_labels
Open

add show_labels function#63
toddyoder-mrg wants to merge 6 commits intometrumresearchgroup:mainfrom
toddyoder-mrg:feature/add-show_labels

Conversation

@toddyoder-mrg
Copy link

New function to view the labels of a data.frame.
Also reran devtools::document() which updated some man files for existing functions.

R/RcppExports.R Outdated
#' @param x A numeric vector of values
#' @param left,right Boundary values
#' @noRd
#' @export
Copy link
Collaborator

Choose a reason for hiding this comment

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

Was this intentional? If not, can we revert this and rerun document to revert the Rd files impacted by this?

Copy link
Author

Choose a reason for hiding this comment

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

Not intentional. I have reverted this, along with the Rd and NAMESPACE files.

Copy link
Author

Choose a reason for hiding this comment

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

@michaelmcd18 I notice now that the top of R/RcppExports.R has this comment:

# Generated by using Rcpp::compileAttributes() -> do not edit by hand

Running Rcpp::compileAttributes() makes this same edit again. I'm not familiar with Rcpp, and I'm not sure how to resolve.

Copy link
Author

@toddyoder-mrg toddyoder-mrg Jan 14, 2026

Choose a reason for hiding this comment

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

I think maybe src/set_bins.cpp and src/auc_p.cpp should each have //' @keywords internal set for the set_bins_cpp and auc_partial_cpp functions respectively (assuming these functions are not meant to be exported by mrgmisc). Does that sound reasonable?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@toddyoder-mrg Agreed on the solution, we don't want either of those exported

Copy link
Author

@toddyoder-mrg toddyoder-mrg Jan 20, 2026

Choose a reason for hiding this comment

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

@michaelmcd18 thanks. I've added //' @keywords internal to both functions and rebuilt the documentation. The CI checks pass now.

fs,
withr
withr,
tidyselect
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you please iterate the package version to 0.3.0.9002 and include yourself as one of the authors of the package @toddyoder-mrg

Copy link
Author

Choose a reason for hiding this comment

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

R/show_labels.R Outdated
# return
tibble::tibble(
name = names(df_meta),
label = stats::setNames(labels, NULL)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this works if you just did label = labels

Copy link
Author

Choose a reason for hiding this comment

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

@michaelmcd18 addressed in d93e63c

Copy link
Collaborator

@michaelmcd18 michaelmcd18 left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks for making all the updates @toddyoder-mrg

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants