Skip to content

Conversation

topepo
Copy link
Member

@topepo topepo commented Aug 17, 2025

Closes #161

When we make a new generic for fit(), it does not work when the S3 method for fit() is available. This PR just re-exports generics::fit() to solve the issue.

@topepo topepo marked this pull request as ready for review August 17, 2025 17:20
Copy link

@Copilot 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 addresses issue #161 by re-exporting the fit() and required_pkgs() generics from the generics package instead of defining custom generics within the package. This ensures compatibility when the generics package is loaded.

  • Removes custom generic definitions for fit() and required_pkgs()
  • Re-exports these generics from the generics package
  • Adds test to verify fit() works correctly when generics package is loaded

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
R/filtro-package.R Adds re-exports of fit and required_pkgs from generics package
R/class_score.R Removes custom generic definitions for fit and required_pkgs
DESCRIPTION Adds generics package as a dependency
NAMESPACE Updates imports to include generics functions
tests/testthat/test-score-cor.R Adds test case to verify fit() works with generics package loaded
man/ files Updates documentation to reflect re-exported generics
tests/testthat/_snaps/score-cor.md Updates error message snapshots

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

@topepo topepo requested a review from franceslinyc August 17, 2025 17:37
@topepo topepo changed the title USe generics::fit() Use generics::fit() Aug 17, 2025
Copy link
Member

@EmilHvitfeldt EmilHvitfeldt 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! tested it many times and it appears to work

@franceslinyc franceslinyc merged commit 233a07e into main Aug 19, 2025
13 checks passed
@franceslinyc franceslinyc deleted the rexport-fit branch August 19, 2025 17:19
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.

conflicting S7 and S3 generics
3 participants