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

print arrays of Bool as 0/1 #30575

Merged
merged 8 commits into from
Jan 9, 2019
Merged

Conversation

stevengj
Copy link
Member

@stevengj stevengj commented Jan 3, 2019

As discussed in #30298, this PR changes the printing of Bool arrays to output 0 and 1 instead of true and false. Rationale:

  • The fact that the values are Bool is already printed in the Array type.
  • 0 and 1 is more compact, and is equally correct since Bool <: Number. Correspondingly, it is arguably more readable for a large number of Bool values.
  • true and false values are confusing in an algebraic context where users are expecting numeric arrays. (This will be exacerbated by Overload UniformScaling to make I(n) create a Diagonal matrix. #30298 if people start using I(n) to construct identity matrices.)

Technically, a Bool value is printed as 0 or 1 by this PR whenever the container has set :typeinfo=Bool in the IOContext, indicating that the Bool type has already been printed. So, for example, true and false are still printed for Any[true, false].

Example:

###### before this PR ######
julia> Matrix(I, 3,4)
3×4 Array{Bool,2}:
  true  false  false  false
 false   true  false  false
 false  false   true  false

julia> Diagonal(I, 3)
3×3 Diagonal{Bool,Array{Bool,1}}:
  true            
        true      
              true

###### after this PR ######
julia> Matrix(I, 3,4)
3×4 Array{Bool,2}:
 1  0  0  0
 0  1  0  0
 0  0  1  0

julia> Diagonal(I, 3)
3×3 Diagonal{Bool,Array{Bool,1}}:
 1    
   1  
     1

@stevengj stevengj added the display and printing Aesthetics and correctness of printed representations of objects. label Jan 3, 2019
@stevengj
Copy link
Member Author

stevengj commented Jan 3, 2019

Apparently unrelated AppVeyor failures? LinearAlgebra\\src\\blas.jl", 92, MethodError(getfield(Libdl, Symbol("#kw##dlsym"))(), ((throw_error = false,), Libdl.dlsym, nothing, :openblas_set_num_threads), 0x00000000000045d5)))) ...?

@lstagner
Copy link
Contributor

lstagner commented Jan 3, 2019

Alternatively, you could go the fortran route and print arrays of booleans as T or F. This might prevent some confusion.

@stevengj
Copy link
Member Author

stevengj commented Jan 3, 2019

@lstagner, besides being confusing in an algebraic context, a Fortran style is a non-starter because it doesn't parse/eval. eval(Meta.parse("Bool[0, 1]")) works just fine, in contrast.

@ararslan
Copy link
Member

ararslan commented Jan 3, 2019

The AppVeyor failure is #30539.

My objection to this is that Bool, despite being a Number, has its own characteristics, e.g. being the only thing that can be put in a conditional. Printing it as an integer seems to suggest that integers could be used in those contexts, which (thankfully) they cannot. While I agree that getting an array of trues and falses looks weird for linear algebra, I think this has the potential to be more confusing from a language learning standpoint.

@StefanKarpinski
Copy link
Member

Ahh...

julia> rand(Bool, 10, 10)
10×10 Array{Bool,2}:
 0  1  0  0  0  1  1  0  1  0
 0  1  0  0  1  0  0  0  1  1
 0  1  0  1  1  0  0  0  1  0
 1  0  1  0  1  1  0  1  0  1
 1  0  1  1  1  1  1  0  1  1
 0  1  1  0  0  1  0  0  1  1
 0  0  0  1  0  1  1  1  1  1
 1  0  0  1  0  1  0  0  0  1
 1  0  0  1  1  1  0  1  0  1
 1  1  1  1  0  0  0  0  1  0

That's so much less noisy than before 😁

@ararslan
Copy link
Member

ararslan commented Jan 4, 2019

If anything, I think that example further underscores the confusion: You're asking for Bool and getting out literal 1s and 0s, which are not Bools cannot be used as Bools.

@StefanKarpinski
Copy link
Member

StefanKarpinski commented Jan 4, 2019

Bools are 1s and 0s. They are just 1s and 0s of a particular bit size which, like various other types of integers in Julia, have special input syntax and are printed differently in some context but not others.

@ararslan
Copy link
Member

ararslan commented Jan 4, 2019

I mean that they aren't Bools in the sense of #30575 (comment).

@StefanKarpinski
Copy link
Member

But your claim was that Bools aren't 1s and 0s, which is not true. Yes, the bare literals 1 and 0 are not Bools, but they equally aren't Int8s and no one objects to printing Int8(1) and Int8(0) as "1" and "0". This is precisely the same thing.

@ararslan
Copy link
Member

ararslan commented Jan 4, 2019

I misspoke; I've corrected the comment. I mean that literal ones and zeros cannot be used like Bools.

but they equally aren't Int8s and no one objects to printing Int8(1) and Int8(0) as "1" and "0". This is precisely the same thing.

My point is that it isn't the same thing, because Bools are special in that they're the only things that can be used in conditionals. So printing a Bool as 0 or 1 suggests that they're interchangeable in a way that they are not.

@stevengj
Copy link
Member Author

stevengj commented Jan 4, 2019

I doubt that printing a Boolean array is something that usually comes up in the first few minutes of using Julia (except for linear algebra demos where one might print I(n) early on!). Scalar boolean printing is not changed here.

In consequence, I would lean toward optimizing the printing of Bool arrays for slightly more experienced users.

@StefanKarpinski
Copy link
Member

I tend to think that this will likely help people understand how Bools work in Julia by helping them realize that Bools are numbers and that false == 0 and true == 1.

Before this PR, with `x = rand(Bool, 10^6)`, `@time repr(x)` took `0.067077 seconds (43 allocations: 8.003 MiB)`.  With the previous version of this PR, I got `0.230414 seconds (2.00 M allocations: 95.556 MiB, 4.60% gc time)` due to the type instability.  After this commit I get `0.047411 seconds (43 allocations: 4.003 MiB)`.
@stevengj
Copy link
Member Author

stevengj commented Jan 4, 2019

Updated to fix a type instability, and performance now seems good. With x = rand(Bool, 10^6), before this PR I got

julia> @time repr(x);
  0.067077 seconds (43 allocations: 8.003 MiB)

and now I get

julia> @time repr(x);
  0.044706 seconds (43 allocations: 4.003 MiB)

so the cost of the extra branch is outweighed by the shorter output.

Copy link
Member

@rfourquet rfourquet left a comment

Choose a reason for hiding this comment

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

I have wanted this change for a long time, didn't expect it would be popular!

test/show.jl Show resolved Hide resolved
test/show.jl Outdated Show resolved Hide resolved
@pablosanjose
Copy link
Contributor

I personally have no problem with this PR, but if the problem is that 0 and 1 are Int literals instead of Bools, one could always show using ⚬ and |, for example, or some other Unicode characters resembling 1 and 0?

@stevengj
Copy link
Member Author

stevengj commented Jan 4, 2019

@pablosanjose, see my comment above: #30575 (comment) — we want to preserve the property that repr parses and evaluates.

@stevengj
Copy link
Member Author

stevengj commented Jan 6, 2019

Whoops, had to update all of the doctests with bool-array output.

@timholy
Copy link
Member

timholy commented Jan 6, 2019

How about

julia> bitrand(2,3)
2×3 BitArray{2}:
 t  t  t
 f  t  t
where t=true, f=false

@o314
Copy link
Contributor

o314 commented Jan 9, 2019

Would be nice if an optional truechar,falsechar pairs could be supplied.
This could permit to render something like this

julia> rand(Bool, 10, 10)
10×10 Array{Bool,2}:
 -  x  -  -  -  x  x  -  x  -
 -  x  -  -  x  -  -  -  x  x
 -  x  -  x  x  -  -  -  x  -
 x  -  x  -  x  x  -  x  -  x
 x  -  x  x  x  x  x  -  x  x
 -  x  x  -  -  x  -  -  x  x
 -  -  -  x  -  x  x  x  x  x
 x  -  -  x  -  x  -  -  -  x
 x  -  -  x  x  x  -  x  -  x
 x  x  x  x  -  -  -  -  x  -

Not only useful for playing naval battle game, but to convey easy to read info to business people.
The principle of double entry table, a generalization of contengency table, pivot table.

This behavior may however better belong to the julia print family than to the show part.
So may be a complementary question is :
Is there anyway to call a print(bools, truechar='x', falsechar='-') to get this?

@stevengj
Copy link
Member Author

stevengj commented Jan 9, 2019

@o314, if you want that display for A = rand(Bool, 10, 10), you can do

julia> [Text(a ? '-' : 'x') for a in A]
10×10 Array{Text{Char},2}:
 -  -  -  -  -  x  x  -  x  x
 -  x  x  -  -  -  -  -  x  x
 x  x  x  x  x  -  -  -  -  -
 -  x  -  -  x  x  -  x  -  x
 -  -  x  -  x  -  -  x  -  x
 x  x  -  x  x  x  x  -  x  -
 x  x  x  x  x  -  -  -  x  x
 -  x  -  x  -  -  x  x  x  x
 -  x  -  -  x  x  x  x  -  -
 -  -  x  -  -  x  -  -  x  -

(Using Text here suppresses the quotation marks.)

@o314
Copy link
Contributor

o314 commented Jan 9, 2019

This could have be done easily with a comprehension of course,
but reinforcing interface is a great purpose too.

Second point Text does not appear clearly at http://docs.julialang.org, only the macro @text_str is stated.
Should we consider it as a stable api in the future?
So with the information accessible there, we could have done

[a ? text"-" : text"x" for a in A]
A = rand(Bool, 3,3)

But that lead to an overwhelming proliferation of api/syntax around the io landscape.
IMHO multiplying syntax shortcut is not a good practice.

As a side effect/near effect, i'm fighting these hours to get a decent table display (think psql or dataframe) without having to load all the dataframe stuff. It will take hours to locate the code - and as much, i guess to cut excessive dependency too.

A question of timeframe i hope

@StefanKarpinski StefanKarpinski merged commit 19fedfb into JuliaLang:master Jan 9, 2019
@stevengj stevengj deleted the boolprint branch January 9, 2019 18:25
@stevengj
Copy link
Member Author

stevengj commented Jan 9, 2019

@o314, yes, Text is exported and documented, and hence should be regarded as a stable API.

@o314
Copy link
Contributor

o314 commented Jan 9, 2019

Thanks for your feedback.

@karajan9
Copy link
Contributor

I get that some people are not 100% sold on this, which is why we are merging it early in the 1.2 cycle as an experiment to see how it feels for a few months. If it causes more confusion than it makes things better, it is easy to revert.

(Without wanting to dive into the pros and cons of this particular proposal...)
I'm wary of this idea in the context of making things more difficult for beginners. The people who would be most confused by the change likely won't be hit by it until 1.2 is out in the world and it is "too late" to change back.
This approach has a similar feel to me to the global-scope-thing where almost all of the feedback came too late because the number of beginners of Julia using nightly builds is quite small.
Of course this change in no way has the same implications nor would it be terribly difficult to change back -- still I think in principle this course of action has a good likelihood to miss its mark.

@JeffBezanson
Copy link
Member

More to the point, I would say printing booleans as 0 and 1 is just not confusing, period.

I'm not sure what alternative you're suggesting --- never make a change that could possibly "confuse" a beginner, because there's no way to get data on it until it's too late?

@stevengj
Copy link
Member Author

@karajan9, see also my comment above.

@StefanKarpinski
Copy link
Member

StefanKarpinski commented Jan 10, 2019

We appreciate the feedback. Personally, I find this change a breath of fresh air every time I encounter it. I didn't realize how much those long, verbose true and false literals in arrays were bothering me. I keep doing rand(Bool, 10, 10) just to see the lovely, terse, clear result. As I've stated in this thread, I think that this provides an excellent teaching moment for users who may be momentarily confused: in Julia, Bool <: Integer with true == 1 and false == 0. If that were not the case, then I agree this would be a terrible change. However, since that is the case this change is not only much better for display but it also teaches the user something important about how the language works. They also learn that they can write Bool[0, 1, 1, 1, 0, 1, 0, 0] instead of Bool[false, true, true, true, false, true, false, false] which is a fairly useful, practical piece of working knowledge.

@StefanKarpinski
Copy link
Member

Also note that @stevengj, who is very strongly in favor of this change and has more experience teaching students Julia than anyone (with the possible exception of @alanedelman), is the only person who pushed back on the global scope change. Take that for what it's worth, but I would be a lot more worried about this if both @stevengj and @JeffBezanson weren't also fully in favor.

@karajan9
Copy link
Contributor

Thanks for the replies! As I said the comment wasn't about this change in particular. Not only because I think I personally will like the change very much but also because I'm not really sure myself it complicates matters that much (and finally, as Jeff hinted to, because more drastic methods like discussing this to death, releasing test versions or something similar are not really warranted here).

I think nobody in this thread suggested they, personally, wouldn't appreciate the change but they were worried about the consequences for beginners of Julia or programming in general. My point was: if that's the concern an approach like "let's just try it and see if someone objects" isn't really practicable and might come across as neglecting of the issue. To avoid a worst case outcome of everybody screaming bloody murder after the fact, statements like

As I've stated in this thread, I think that this provides an excellent teaching moment for users who may be momentarily confused: in Julia, Bool <: Integer with true == 1 and false == 0. [...]

and

Also note that @stevengj, who is very strongly in favor of this change and has more experience teaching students Julia than anyone [...]

are much more helpful and satisfactory, I think.

@tkf
Copy link
Member

tkf commented Jan 10, 2019

rand(Bool, 10, 10) looks really nice. However, I find the following a bit confusing:

julia> [(true, 0), (false, 1), (true, 2), (false, 3)]
4-element Array{Tuple{Bool,Int64},1}:
 (1, 0)
 (0, 1)
 (1, 2)
 (0, 3)

Also:

julia> [true => false]
1-element Array{Pair{Bool,Bool},1}:
 1 => 0

julia> Dict(true => false)  # inconsistent with above?
Dict{Bool,Bool} with 1 entry:
  true => false

julia> show(Dict(true => false))  # needs Dict{Bool,Bool}?
Dict(1=>0)

It looks like display(Dict(...)) does not set typeinfo while show(Dict(...)) does.

(Edit: I re-reported Dict part in #30683)

@StefanKarpinski
Copy link
Member

StefanKarpinski commented Jan 10, 2019

Definitely needs to be smoothed out a bit, thanks for filing the Dict issue!

@o314
Copy link
Contributor

o314 commented Jan 13, 2019

@test BitSet(Bool[0, 1, 1, 1, 0, 1, 0]) == BitSet([0, 1])
Test Passed

isn't a bit confusing now ?

show(::IO,::BitSet) could be reviewed too...

@stevengj
Copy link
Member Author

@o314, that test wasn't changed by this PR, nor was BitSet printing.

@diegozea
Copy link
Contributor

I've found this because it makes this test fail on nightly: https://github.com/diegozea/MIToS.jl/blob/a8c4075673bf439382c10592211e778f75527b7f/test/Scripts/Template.jl#L47

Maybe it will be better to define a binary type for LinearAlgebra instead of using Bool. This is at least confusing...

nassarhuda added a commit to nassarhuda/julia that referenced this pull request Sep 12, 2019
Changes to the documentation as bitarrays now display 0s and 1s (JuliaLang#30575)
@nassarhuda nassarhuda mentioned this pull request Sep 12, 2019
fredrikekre pushed a commit that referenced this pull request Sep 13, 2019
Changes to the documentation as bitarrays now display 0s and 1s (#30575)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
display and printing Aesthetics and correctness of printed representations of objects.
Projects
None yet
Development

Successfully merging this pull request may close these issues.