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

[WIP]/[RFC] Move EVEN MORE things out of helpDB #18511

Closed
wants to merge 3 commits into from

Conversation

kshyatt
Copy link
Contributor

@kshyatt kshyatt commented Sep 14, 2016

A bunch of these docstrings are kind of confusing and I don't know the best way to reword. Feedback very welcome!

@kshyatt kshyatt added the docs This change adds or pertains to documentation label Sep 14, 2016
immutable NullException <: Exception
end

"""
Nullable(x, isnull::Bool=false)
Copy link
Contributor

Choose a reason for hiding this comment

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

is the second argument meant to be used?

Copy link
Member

Choose a reason for hiding this comment

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

Let's discuss this at #18510 (comment).

error(message::AbstractString)

Raise an [`ErrorException`](:obj:`ErrorException`) with the given message.
"""
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Wouldn't it be better to document the more general error(s...) below?

"""
systemerror(sysfunc, iftrue::Bool; extrainfo=nothing)

Raises a `SystemError` for `errno` with the descriptive string `sysfunc` if `iftrue` is `true`.
Copy link
Sponsor Member

@KristofferC KristofferC Sep 14, 2016

Choose a reason for hiding this comment

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

This one is quite confusing. What is errno? What does extrainfo do?

assert(cond)

Throw an [`AssertionError`](:obj:`AssertionError`) if `cond` is `false`.
Also available as the macro `@assert expr`.
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Maybe we could add a ref link here to the macro?

"""
isnull(x::Nullable) -> Bool

Is the [`Nullable`](:obj:`Nullable`) object `x` null, i.e. missing a value?
Copy link
Sponsor Member

@KristofferC KristofferC Sep 14, 2016

Choose a reason for hiding this comment

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

I think this would be better as a statement instead of a question.

Returns true if the ...

or something

Copy link
Member

Choose a reason for hiding this comment

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

Or "Determine whether the Nullable object x is null, i.e. is missing."

Copy link
Member

@nalimilan nalimilan Sep 15, 2016

Choose a reason for hiding this comment

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

"Determines" (third person, as you recalled my last time :-).

EDIT: looks like that's the opposite. I can never remember this rule...

@ViralBShah
Copy link
Member

The docstrings need some work in general all over. For now, should we move over and merge and then tweak them?


Bessel function of the first kind of order 0, ``J_0(x)``.
"""
function besselj0(x) end
Copy link
Member

Choose a reason for hiding this comment

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

This will define a new method for besselj0 returning nothing which we probably don't want. Should be besselj0(x) without the function and end. Same with the other bessel* and erf* ones below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I fix this up, following what @ViralBShah said, should we merge?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather do some cleanup now while people are looking at the diff

"""
getindex(A, inds...)

Returns a subset of array `A` as specified by `inds`, where each `ind` may be an
Copy link
Member

Choose a reason for hiding this comment

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

I think the preferred style is to describe the action in the imperative. In this case it would be "return" instead of "returns."


Bessel function of the first kind of order 0, ``J_0(x)``.
"""
besselj0(x)
Copy link
Member

Choose a reason for hiding this comment

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

I think these should be besselj0 without the (x)

Copy link
Member

Choose a reason for hiding this comment

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

If there's only a single docstring for besselj0 then either syntax is fine really. It'll only matter if we had several different methods of the function and we wanted to document each one separately.

Compute the error function of `x`, defined by ``\\frac{2}{\\sqrt{\\pi}} \\int_0^x e^{-t^2} dt``
for arbitrary complex `x`.
"""
function erf(x) end
Copy link
Member

Choose a reason for hiding this comment

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

Same comment as with the Bessel functions: should just be erf.

@kshyatt
Copy link
Contributor Author

kshyatt commented Dec 24, 2016

This is so old, and way out of date, let's start again new.

@kshyatt kshyatt closed this Dec 24, 2016
@kshyatt kshyatt deleted the ksh/docamputation2 branch December 24, 2016 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants