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

doctests for the manual, part 3 #20202

Merged
merged 2 commits into from
Jan 27, 2017

Conversation

fredrikekre
Copy link
Member

nothing exciting here, just plain conversion to doctests and some whitespace fixes

(make check-whitespace and the doctests pass locally, remove [ci skip] from commit message if/when this is merged)

@@ -152,27 +150,18 @@ julia> z
10
```

Multiple global or local definitions can be on one line and can also be paired with assignments:
Copy link
Member Author

Choose a reason for hiding this comment

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

this was deprecated in #19905

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 replacement syntax demo'ed in the docs anywhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not that I know of.

Copy link
Contributor

Choose a reason for hiding this comment

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

should it be?

Copy link
Member Author

Choose a reason for hiding this comment

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

It seems that it is only when paired with assignment that is deprecated:

julia> global a, b

julia> global x = 1; d

WARNING: deprecated syntax "global c=1, d" at REPL[2]:1.
Use "global c=1; global d" instead.

It seems a bit overkill to say that if you wanna pair it with assignment the keyword needs to be in front of each variable?

On another note: There is an empty line before the warning which is not the case for other warnings

Copy link
Contributor

Choose a reason for hiding this comment

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

The extra newline should probably be fixed... syntax deprecations get thrown from inside the parser I think. In 1.0 I guess this syntax would be an error, in which case hopefully the error message will indicate the correct syntax in a clear way?

@kshyatt kshyatt added the docs This change adds or pertains to documentation label Jan 23, 2017
julia> sqrt(-1)
ERROR: DomainError:
sqrt will only return a complex result if called with a complex argument. Try sqrt(complex(x)).
in sqrt(::Int64) at ./math.jl:278
Copy link
Contributor

Choose a reason for hiding this comment

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

why are you removing backtrace lines?

Copy link
Member Author

Choose a reason for hiding this comment

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

The point of the example is just to show that it throws a DomainError? And if the backtrace is removed like this the doctest doesn't need to be updated every time something change in math.jl
Should I include backtraces for all examples then? I have omitted it for some other blocks too.

Copy link
Contributor

@tkelman tkelman Jan 24, 2017

Choose a reason for hiding this comment

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

I find at least the first line or two of the backtrace to be pretty useful, it tells you where to go find how the code is implemented if you want to look, and it's representative of what happens when you run this code yourself. The line numbers thing we should address in terms of how we run the doctests and how picky we are about everything matching. I think we should detect line numbers in the output and be permissive of them changing, everywhere except on release-branch builds.

Copy link
Member

Choose a reason for hiding this comment

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

As a counter point I typically find all the backtraces in the docs annoying and noisy.

Copy link
Member Author

Choose a reason for hiding this comment

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

It can be noted that the [...] is actually rendered too so it is implicit that there are things hidden. The above case renders to this:
backtrace

Note also that it is not consistent whether the backtrace is shown or not across the manual. What do other people have to say?

Copy link
Member

Choose a reason for hiding this comment

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

What do other people have to say?

I've got no particular preference for either, largely depends on the purpose of each doctest I think.

x = 2 # assignment introduces a new local
return x + y # y refers to the global
x = 2 # assignment introduces a new local
return x +y # y refers to the global
Copy link
Contributor

Choose a reason for hiding this comment

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

the space between the + and y should be kept

```

Boolean operations *without* short-circuit evaluation can be done with the bitwise boolean operators
introduced in [Mathematical Operations and Elementary Functions](@ref): `&` and `|`. These are
normal functions, which happen to support infix operator syntax, but always evaluate their arguments:

```julia
```@meta
Copy link
Member

Choose a reason for hiding this comment

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

Same remark, again, as in https://github.com/JuliaLang/julia/pull/20183/files#r97537182 can apply here as well I think.

@fredrikekre fredrikekre force-pushed the fe/manual-doctests-part3 branch 2 times, most recently from a8801c8 to fa9e404 Compare January 24, 2017 22:21
@fredrikekre
Copy link
Member Author

I guess no one else seem to be bothered by the stacktraces in the manual, so I will add them back.
But for the record I think that it is clear that the output is truncated from the dots, ref #20202 (comment)

I will update, and add them back.

@fredrikekre fredrikekre force-pushed the fe/manual-doctests-part3 branch from fa9e404 to 788827c Compare January 26, 2017 23:07
@fredrikekre
Copy link
Member Author

Re-added stacktraces, added doctests and check-whitespace pass locally.

@fredrikekre fredrikekre mentioned this pull request Jan 27, 2017
38 tasks
@tkelman tkelman merged commit 8a75199 into JuliaLang:master Jan 27, 2017
@fredrikekre fredrikekre deleted the fe/manual-doctests-part3 branch January 27, 2017 15:19
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.

5 participants