Skip to content

fix mlr3 integration, add light rcmdcheck and fix its complaints#29

Merged
mmyrte merged 27 commits into
mainfrom
copilot/integrate-mlr3-library
May 12, 2026
Merged

fix mlr3 integration, add light rcmdcheck and fix its complaints#29
mmyrte merged 27 commits into
mainfrom
copilot/integrate-mlr3-library

Conversation

@mmyrte
Copy link
Copy Markdown
Member

@mmyrte mmyrte commented May 6, 2026

There were issues I had not thought of in #24

  • consistent select_score/select_maximize terminology
  • make sure pkgdown succeeds
  • mlr3-style split sampling
  • dependencies...

Comment thread R/trans_models_t.R Outdated
on = "id_trans"
]

# TODO put in validator that the learner has predict_type = "prob" set
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@copilot fix this, but instead of erroring out straight away, i want both fit_partial_models and fit_full_models to coerce the learner to mlr3's predict_type "prob"; if that fails, then we should error out. we should also verify at this point that the learner supports twoclass classifications.

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.

Done in 552ceac. Added a coerce_learner_for_classif() internal helper that is called at the top of both fit_partial_models and fit_full_models (direct-learner branch). It:

  1. Checks the learner is an mlr3 Learner/AutoTuner (errors if not).
  2. Checks "twoclass" %in% learner$properties (errors if not).
  3. Coerces predict_type to "prob" if it isn't already — wrapping the assignment in tryCatch so that learners which don't support probabilistic output get a descriptive error instead of mlr3's raw param-set message.

Tests added: auto-coercion from "response" to "prob" (verifying the side-effect on the passed-in R6 object), and an error when twoclass support is absent.

Copilot finished work on behalf of mmyrte May 6, 2026 10:17
@mmyrte mmyrte marked this pull request as draft May 7, 2026 13:59
@mmyrte mmyrte marked this pull request as ready for review May 7, 2026 13:59
@mmyrte mmyrte changed the title Fix: Integrate mlr3 library properly fix mlr3 integration, add light rcmdcheck and fix its complaints May 12, 2026
@mmyrte mmyrte merged commit d776049 into main May 12, 2026
2 checks passed
@mmyrte mmyrte mentioned this pull request May 12, 2026
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