New range display2 #13534

artkuo · 2015-10-10T17:08:48Z

This is the result of a user's forum discussion about ranges. People were unhappy that 0:5 or linspace(1,10,7) does not return a vector, as they wanted. I believe it is sensible for these to be range objects (rather than being converted via collect), but the REPL display could be modified to show what the range actually looks like. The user probably wants to see some confirmation of what they were thinking, and seeing the definition doesn't help.

The proposed solution is to give REPL output like this:

julia> 0:4
5-element UnitRange{Int64}:
 0,1,2,3,4

julia> 0:100
101-element UnitRange{Int64}:
 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,…,89,90,91,92,93,94,95,96,97,98,99,100

julia> linspace(0,2.5,4)
4-element LinSpace{Float64}:
 0.0,0.833333,1.66667,2.5

This is accomplished by overloading writemime (replutil.jl) to respond to range objects, adding a print_range method (range.jl) and appropriate unit tests (test/ranges.jl). This affects display(), but the user can still use show(1:5) to return the definition 1:5. The print_range method is based on the existing print_matrix_row, and so it uses knowledge of screen size to make everything fit in one row, with horizontal dots (ellipsis) where appropriate.

Additional edits were made to show.jl, where I added a bunch of comments. I had to learn how print_matrix works, and the comments are meant to save time for someone like me, as the code is a bit opaque. There were absolutely no changes to execution of show.jl.

A few notes. I decided not to add brackets around the range. Perhaps square brackets would be appropriate, but I felt they were not needed, as a range is not exactly an array. Also, the range is printed as a single row to save vertical screen space. I thought this was acceptable because tuples are also printed across, despite acting somewhat like 1-d arrays, which are kind of "vertical." I would appreciate feedback if there are disagreements about these choices, or about the whole idea of modifying the REPL.

This is take 2: Fixed test to allow for 32- or 64-bit systems; changed pull request to base from master

…s added while trying to figure out print_matrix and related functions.

pao · 2015-10-10T17:59:34Z

cf #13524

stevengj · 2015-10-13T15:04:23Z

base/range.jl

+#  1.0,2.0,3.0,…,4.0,5.0,6.0
+# This function is borrowed from print_matrix in show.jl
+# It is usually called by writemime (from replutil.jl)
+function print_range(io::IO, r::Union{Range,LinSpace},


This Union is unnecessary (similarly in writemime, below), because LinSpace <: Range.

…xported methods such as alignment.

artkuo · 2015-10-13T22:48:41Z

In response to comments, I switched writemime to stringmime in tests, removed the redundant Union{Range,LinSpace}, and changed the code comments I had added into Docstring format.

Also, I discovered that Base.print_matrix is slow when displaying large matrices (try eye(10^4) in REPL), because it examines the spacing of the entire matrix, even when few elements will be printed. My code borrowed from print_matrix, but I have found a faster way and amended my print_range accordingly. This can be demonstrated by typing 0:10^9 at the REPL. I plan to go back and fix print_matrix as well and submit a separate pull request.

Thanks stevengj for suggesting helpful fixes.

stevengj · 2015-10-13T23:09:07Z

base/range.jl

+`pre` and `post` characters for each printed row, `sep` separator string between
+printed elements, `hdots` string for the horizontal ellipsis.
+"""
+"""


docstring is duplicated?

I wouldn't say duplicated, it's just that I'm not a huge fan of f(a, [b, c]) to show optional parameters. I think it's easier for a reader to parse the simpler call first, and then see how additional parameters are added on. If the square brackets are considered the standard, I can switch to that, but otherwise I prefer my docstring.

Look again. You have the whole docstring twice.

doh! sorry about that; removed.

stevengj · 2015-10-14T02:13:27Z

Hmm, didn't it used to pass the tests? What changed?

artkuo · 2015-10-14T02:34:22Z

This is very strange. I deleted the extra docstring, and suddenly it wouldn't compile. I was able to move the docstring below the function definition, and it successfully makes and tests clean. I do not understand why it would work with a double docstring, and not with a single. I have no idea whether the bug was mine or elsewhere, but except for the odd location of the docstring, everything seems to work now.

stevengj · 2015-10-14T02:41:11Z

base/range.jl

@@ -244,6 +244,75 @@ function show(io::IO, r::LinSpace)
    print(io, ')')
 end

+# `print_range(io, r)` prints out a nice looking range r in terms of its elements


don't duplicate the docstring in a comment. Just say # See docstring below.

stevengj · 2015-10-14T02:42:47Z

That's very odd. @one-more-minute, could there be a parser/compiler bug here? I don't see how duplicating the docstring could (correctly) change it from failure to success.

MichaelHatherly · 2015-10-14T06:08:00Z

suddenly it wouldn't compile.

@artkuo, was this the error you were getting?

LoadError(at "sysimg.jl" line 39: LoadError(at "range.jl" line 299: Base.MethodError(f===, args=(:function, :call))))
...

If so, I'll have a PR to fix this shortly.

I don't see how duplicating the docstring could (correctly) change it from failure to success.

When the docstring was duplicated the first docstring was documenting the second one so the function wasn't being documented at all. See #12737.

Docstrings defined before `==` is available for `Symbol` comparisions causes a build error. Fix by comparing with `===`. Ref: JuliaLang#13534 (comment)

Docstrings defined before `==` is available for `Symbol` comparisons causes a build error. Fix by comparing with `===`. Ref: JuliaLang#13534 (comment)

…10787

Revert "Update types.rst"

artkuo · 2015-10-14T15:29:01Z

That was indeed the error. I found out that during the build process, any docstrings in files compiled before operators.jl (line 11) would cause this error. Line 11 reads: ==(x,y) = x === y which I believe is addressed by #13602. And of course, ranges.jl is one of about a dozen files that come before operators.jl.

stevengj · 2015-10-14T16:14:14Z

Probably best to wait for #13602 to be merged, and then put the docstring back before the function, and finally squash the commits. Then I think it should be ready to merge.

…rap-fix Fix doc bootstrap method error.

Deprecate A_ldiv_B!(SparseMatrixCSC, StridedVecOrMat) thereby fixing #10787

…ructors Make the constructors of Array and SharedArray consistent

…e in REPL. Previous commits squashed, docstring moved back to front, builds and passes tests

artkuo · 2015-10-14T21:03:41Z

Had to pull #13602 to test, but didn't know how to push back to this branch without including all the commits that had happened upstream. Closing this and starting a new one that should be clean. See #13615 for clean version.

KristofferC · 2015-10-14T21:08:57Z

You can do git rebase master from your branch and it tries to put your commits on top of the newest commit on master.

Docstrings defined before `==` is available for `Symbol` comparisons causes a build error. Fix by comparing with `===`. Ref: #13534 (comment) (cherry picked from commit d8b6a24) ref #13602

polarke and others added 4 commits October 9, 2015 17:36

add same constructors as array

a2aabfa

display ranges with their elements

2298652

Code comments for show.jl; no changes to execution code, only comment…

19ddb5f

…s added while trying to figure out print_matrix and related functions.

make sure test works on either 32- or 64-bit systems (Int32 or Int64)

6acb510

add test for sharedarray constructor

8e98763

stevengj reviewed Oct 13, 2015
View reviewed changes

artkuo added 2 commits October 13, 2015 18:20

improved print_range, tweaked comments and tests

721350e

Docstring comments for print_X methods in show. Comments are for un-e…

690bc2b

…xported methods such as alignment.

streamlined writemime range type

2b3f95d

stevengj reviewed Oct 13, 2015
View reviewed changes

artkuo added 2 commits October 13, 2015 20:16

more concise tests

f81b4f6

removed unwanted dup of docstring

2d445e0

artkuo mentioned this pull request Oct 14, 2015

Fix Base.print_matrix() for big matrices #13598

Merged

moved docstring below, due to strange problem with compilation

5e05afe

stevengj reviewed Oct 14, 2015
View reviewed changes

MichaelHatherly mentioned this pull request Oct 14, 2015

Fix doc bootstrap method error. #13602

Merged

MichaelHatherly and others added 3 commits October 14, 2015 08:35

Fix doc bootstrap method error.

d8b6a24

Docstrings defined before `==` is available for `Symbol` comparisons causes a build error. Fix by comparing with `===`. Ref: JuliaLang#13534 (comment)

Deprecate A_ldiv_B!(SparseMatrixCSC, StrideVecOrMat) thereby fixing #…

87131f3

…10787

Merge pull request JuliaLang#13608 from JuliaLang/revert-13604-patch-1

7408142

Revert "Update types.rst"

tkelman and others added 5 commits October 14, 2015 09:58

Merge pull request JuliaLang#13602 from MichaelHatherly/mh/doc-bootst…

052f900

…rap-fix Fix doc bootstrap method error.

Merge pull request JuliaLang#13496 from JuliaLang/anj/ldiv

ecf6005

Deprecate A_ldiv_B!(SparseMatrixCSC, StridedVecOrMat) thereby fixing #10787

Merge pull request JuliaLang#13514 from polarke/fix-sharedarray-const…

d35e69c

…ructors Make the constructors of Array and SharedArray consistent

added print_range for Range objects, to show the elements of the Rang…

bd30ace

…e in REPL. Previous commits squashed, docstring moved back to front, builds and passes tests

hi

601b0a9

artkuo closed this Oct 14, 2015

artkuo mentioned this pull request Oct 14, 2015

added print_range for Range objects, to show the elements of the Rang… #13615

Merged

MichaelHatherly mentioned this pull request Oct 15, 2015

Second of two consecutive strings is ignored #12737

Closed

artkuo deleted the new_range_display2 branch November 12, 2015 19:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New range display2 #13534

New range display2 #13534

artkuo commented Oct 10, 2015

pao commented Oct 10, 2015

stevengj Oct 13, 2015

artkuo commented Oct 13, 2015

stevengj Oct 13, 2015

artkuo Oct 14, 2015

stevengj Oct 14, 2015

artkuo Oct 14, 2015

stevengj commented Oct 14, 2015

artkuo commented Oct 14, 2015

stevengj Oct 14, 2015

stevengj commented Oct 14, 2015

MichaelHatherly commented Oct 14, 2015

artkuo commented Oct 14, 2015

stevengj commented Oct 14, 2015

artkuo commented Oct 14, 2015

KristofferC commented Oct 14, 2015

New range display2 #13534

New range display2 #13534

Conversation

artkuo commented Oct 10, 2015

pao commented Oct 10, 2015

stevengj Oct 13, 2015

Choose a reason for hiding this comment

artkuo commented Oct 13, 2015

stevengj Oct 13, 2015

Choose a reason for hiding this comment

artkuo Oct 14, 2015

Choose a reason for hiding this comment

stevengj Oct 14, 2015

Choose a reason for hiding this comment

artkuo Oct 14, 2015

Choose a reason for hiding this comment

stevengj commented Oct 14, 2015

artkuo commented Oct 14, 2015

stevengj Oct 14, 2015

Choose a reason for hiding this comment

stevengj commented Oct 14, 2015

MichaelHatherly commented Oct 14, 2015

artkuo commented Oct 14, 2015

stevengj commented Oct 14, 2015

artkuo commented Oct 14, 2015

KristofferC commented Oct 14, 2015