Skip to content

Conversation

@mhovd
Copy link
Collaborator

@mhovd mhovd commented Dec 16, 2025

If the user simply uses ?fit, they are redirected to the Rust function.

I wonder if this function should not be made public, as it is not meant for users to call directly.

Copilot AI review requested due to automatic review settings December 16, 2025 10:58
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves the documentation for the internal fit function by adding a note that it should not be called directly and providing a cross-reference to the public PM_model$fit() method. This addresses user confusion when using ?fit, which currently redirects to the low-level Rust function instead of the intended public API.

Key Changes

  • Added documentation notes indicating fit is an internal function
  • Added cross-reference to PM_model$fit() method for users seeking help
  • Updated both Rust source and R wrapper documentation

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/rust/src/lib.rs Added roxygen2-style documentation comments to the Rust fit function noting it's internal and referencing PM_model$fit()
R/extendr-wrappers.R Added roxygen2 documentation to the R wrapper for fit with internal function note and cross-reference
man/fit.Rd Generated documentation file updated with the new internal function notice and cross-reference
Comments suppressed due to low confidence (1)

R/extendr-wrappers.R:61

  • Since this is documented as an internal function that should not be called directly, it should include the @keywords internal tag for consistency with other internal exported functions in the codebase (e.g., see setup_logs at line 77-79). Add #' @keywords internal before the @export tag.
#' Fits the model at the given path to the data at the given path using the provided parameters.
#' NOTE: This is an internal function and should not be called directly.
#' See [PM_model] `$fit()` method for arguments.
#' @param model_path Path to the compiled model file.
#' @param data Path to the data file.
#' @param params List of fitting parameters.
#' @param output_path Path to save the fitting results.
#' @param kind Kind of model, which can either be "ODE" or "Analytical".
#' @return Result of the fitting process.
#' @export

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

mhovd and others added 2 commits December 17, 2025 10:18
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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