Skip to content

Conversation

@mboecker
Copy link
Contributor

As we discussed @jakob-r I tried to convert our mlr3 docs to the R6 Roxygen docs. I'm not sure if "@Usage NULL" should still be needed? I included it because otherwise the usage section would just say "LearnerClassif".

@codecov-io
Copy link

codecov-io commented Nov 10, 2019

Codecov Report

Merging #399 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #399   +/-   ##
=======================================
  Coverage   91.69%   91.69%           
=======================================
  Files          71       71           
  Lines        1783     1783           
=======================================
  Hits         1635     1635           
  Misses        148      148
Impacted Files Coverage Δ
R/LearnerRegr.R 100% <ø> (ø) ⬆️
R/LearnerClassif.R 100% <ø> (ø) ⬆️
R/Learner.R 92.59% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 957f059...6552ebc. Read the comment docs.

Copy link
Member

@pat-s pat-s left a comment

Choose a reason for hiding this comment

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

Looks not bad! And it seems the changes are not so big.

Unrelated to the changes of this PR, there is a linking issue in the Description section.

2019-11-10-185104_screenshot

Also the "Method" section has a subsection which can be toggled interactively - to reveal the inherited methods. Nice tweak of RStudio.

2019-11-10-185307_screenshot

@mllg
Copy link
Member

mllg commented Nov 16, 2019

This is not really converted to the new documentation approach in roxygen2 v7. Have you read https://roxygen2.r-lib.org/articles/rd.html#r6 ?

@mboecker
Copy link
Contributor Author

Thanks for the link I havent seen that documentation. I have worked off the inital issue for R6 support in roxygen (r-lib/roxygen2#922) but since there is actual documentation, I will gladly take another look at it.

@mboecker
Copy link
Contributor Author

the new commit should actually document this using the new R6 syntax. sorry for my misunderstanding. I documented the parameters of $new by itself in this file but they would usually documented as inherited parameters from Learner.

@pat-s
Copy link
Member

pat-s commented Dec 3, 2019

Can you give a quick summary on the current status?

@jakob-r
Copy link
Member

jakob-r commented Dec 3, 2019

@mboecker is still working on it.

@mllg
Copy link
Member

mllg commented Feb 20, 2020

Superseded by #439.

@mllg mllg closed this Feb 20, 2020
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.

5 participants