Conversation
There was a problem hiding this comment.
Code Review
This pull request updates documentation URLs, adds the GPL-3 license, and introduces the VariableImportance S7 class along with new importance measures for GAM models. Feedback recommends removing a leftover reference to Copilot instructions in the contribution guide for consistency and suggests implementing a formal test case for the new GAM variable importance functionality.
I am having trouble creating individual review comments. Click here to see my feedback.
.github/CONTRIBUTING.md (30)
The reference to .github/copilot-instructions.md was removed from the 'Review Documentation' section here, but it is still mentioned on line 164 of this file. For consistency, the reference at the end of the document should also be removed or updated if those instructions are no longer intended for use by contributors.
tests/testthat/test_Supervised.R (1366-1371)
The removed GAM test code was redundant and located outside a test_that block. However, since the NEWS.md mentions a new VariableImportance feature for GAM models, this is a good opportunity to add a proper test case to verify that the new importance measure is correctly calculated and returned.
test_that("GAM variable importance works", {
x <- rnorm(100)
y <- x^3 + rnorm(100)
mod <- train(data.table(x, y), algorithm = "gam", verbosity = 0)
expect_s7_class(mod@varimp, VariableImportance)
expect_true("Partial_Effect_Variance" %in% names(mod@varimp@data))
})There was a problem hiding this comment.
Pull request overview
This PR updates repository/package documentation pointers and release artifacts to reflect the new docs site structure, while doing some small housekeeping around metadata, ignores, and tests.
Changes:
- Update documentation URLs across package docs/metadata and startup messaging to
https://docs.rtemis.org/r/ml(+ API to/r/ml-api/). - Add release notes (
NEWS.md) and include the GPLv3 text inLICENSE.md(excluded from builds via.Rbuildignore). - Minor cleanup: remove stray non-test code from a test file, add
specs/to.gitignore, refreshDESCRIPTIONdate, and update contributing install instructions.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
tests/testthat/test_Supervised.R |
Removes stray ad-hoc code at the end of the test file. |
man/rtemis-package.Rd |
Updates package help links to the new docs paths. |
R/rtemis-package.R |
Updates roxygen source for the package docs URL. |
R/zzz.R |
Updates the startup “Docs” link shown on attach. |
DESCRIPTION |
Updates Date and URL fields to the new docs endpoints. |
.github/CONTRIBUTING.md |
Updates install instructions (CRAN/GitHub/r-universe) and documentation links. |
README.md |
Adds a copyright/license footer. |
NEWS.md |
Adds release notes for 1.0.0 and 1.0.1. |
LICENSE.md |
Adds the full GPLv3 license text. |
.gitignore |
Ignores specs/. |
.Rbuildignore |
Excludes LICENSE.md and specs from package builds. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| --- | ||
|
|
||
| © 2016–2026 E.D. Gennatas. Licensed under [GPL (>= 3)](https://www.gnu.org/licenses/gpl-3.0.html). | ||
|
|
There was a problem hiding this comment.
The README still links to the old docs URL (e.g. https://docs.rtemis.org/r/ in the docs badge near the top and in the “Documentation” section). Since this PR migrates docs links to /r/ml, these should be updated too to avoid sending users to the wrong site.
|
|
||
| ## 1.0.1 | ||
|
|
||
| - Introduce `VariableImportance` S7 class to represent variable importance data, allowing for more than one measure of importance per model and update all relevant classes and methods. |
There was a problem hiding this comment.
NEWS entry has a grammar issue: “allowing for … per model and update all …” mixes verb forms and reads incorrectly. Rephrase to keep tense consistent (e.g., “...and updates...” or “..., updating ...”).
| - Introduce `VariableImportance` S7 class to represent variable importance data, allowing for more than one measure of importance per model and update all relevant classes and methods. | |
| - Introduce `VariableImportance` S7 class to represent variable importance data, allowing for more than one measure of importance per model and updating all relevant classes and methods. |
| ## 1.0.1 | ||
|
|
||
| - Introduce `VariableImportance` S7 class to represent variable importance data, allowing for more than one measure of importance per model and update all relevant classes and methods. | ||
| - Calculate Partial_Effect_Variance as variable importance measure for GAM models |
There was a problem hiding this comment.
Consider formatting Partial_Effect_Variance as inline code and adding consistent punctuation (e.g., ending the bullet with a period) to match the surrounding NEWS style.
| - Calculate Partial_Effect_Variance as variable importance measure for GAM models | |
| - Calculate `Partial_Effect_Variance` as variable importance measure for GAM models. |
No description provided.