Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Documentation Additions to kde #28

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

benholmgren
Copy link
Contributor

Specified parameters with roxygen style comments, code functionality also outlined.

benholmgren and others added 2 commits January 30, 2019 14:16
Specified parameters with roxygen style comments, code functionality also outlined
Copy link
Contributor

@dlm dlm left a comment

Choose a reason for hiding this comment

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

Very minor comments. Please address and we will merge in at the meeting on Mon.

R/kde.R Outdated
@@ -26,7 +34,7 @@ function(X, Grid, h, kertype = "Gaussian", weight = 1, printProgress = FALSE) {
if (!is.logical(printProgress)) {
stop("printProgress should be a logical variable")
}

# utilize Kde function with inputs
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove comment. Generally, it is better to use comments to explain code that is hard to understand. In this case, the comment doesn't provide any additional value.

R/kde.R Outdated
function(X, Grid, h, kertype = "Gaussian", weight = 1, printProgress = FALSE) {

# exception handling of inputs to ensure functionality
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's remove also for a somewhat similar comment to below. Although this comment does provide some additional information to the actual code, almost all of the functions in the package do some precondition checking. Since each function is doing some checks, we would end up having the same comments everywhere.

Removed unnecessary comments for kde function as requested
Roxygen comments have to be outside function, and needed a title to render properly. Added return value as well.
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