Keywords for string, deprecate bin, oct, dec, hex, base #25717

bramtayl · 2018-01-24T01:28:26Z

No description provided.

bramtayl · 2018-01-24T01:29:40Z

Based on the advice here: #25188 (comment)

JeffBezanson · 2018-01-24T03:31:37Z

HISTORY.md

@@ -417,7 +417,7 @@ Library improvements
    respectively perform predicate function negation and function composition. For example,
    `map(!iszero, (0, 1))` is now equivalent to `map(x -> !iszero(x), (0, 1))` and
    `map(uppercase ∘ hex, 250:255)` is now equivalent to
-    `map(x -> uppercase(hex(x)), 250:255)` ([#17155]).
+    `map(x -> uppercase(base(16, x)), 250:255)` ([#17155]).


I think this file should be left alone --- no need to try to rewrite history :)

Don't tell Howard Zinn

JeffBezanson · 2018-01-24T03:38:07Z

base/libgit2/oid.jl

+    assert_sixteen(b)
+    join([base(b,i,2) for i in id.val])
+end
+function Base.base(b, id::GitShortHash)


I'm not sure these methods should exist (same applies to the hex methods, which I was unaware of). Converting a GitHash to a string does the same thing.

Yeah, let's just deprecate hex in favour of string.

JeffBezanson

Should also be removed from exports.jl

JeffBezanson · 2018-01-24T04:21:41Z

base/intfuncs.jl

-```
-"""
-dec
+base(b::Integer, n::Char; pad::Integer = 1) = base(b, UInt32(n); pad = pad)


I think this should be removed (i.e. deprecated). Might as well require explicitly calling UInt32(c).

simonbyrne · 2018-01-24T17:24:35Z

base/libgit2/oid.jl

 """
    raw(id::GitHash) -> Vector{UInt8}

 Obtain the raw bytes of the [`GitHash`](@ref) as a vector of length $OID_RAWSZ.
 """
 raw(id::GitHash) = collect(id.val)

-Base.string(id::AbstractGitHash) = hex(id)
+Base.string(id::AbstractGitHash) = base(16, id)


Is there a base(::Int, ::AbstractGitHash) method defined somewhere?

What I meant in my previous comment was to define:

Base.string(id::GitHash) = join([base(i,2,pad=2) for i in id.val]) Base.string(id::GitShortHash) = string(id.hash)[1:id.len]

Oh got it I thought you meant these were already defined

This should actually be a method of print, then you also get string for free.

We usually define string when it's actually easier to compute the String representation than to print it. This is quite rare, but if it can be implemented by calling base, this would be one of those cases.

simonbyrne · 2018-01-24T17:26:13Z

Would it make sense to also provide some option to pad to the full width of the type (say pad=true), so that base(16,::UInt8,pad=true) is always 2 characters?

JeffBezanson · 2018-01-24T18:16:12Z

Let's avoid adding new features for now.

StefanKarpinski · 2018-01-24T18:49:45Z

Deprecating bin, oct, dec and hex is good – those were way too short and specific to be exported from Base. Perhaps base(b, n, pad=p) should be spelled string(n, base=b, pad=p)?

bramtayl · 2018-01-24T19:26:34Z

That sounds better to me, especially because

string(x::Union{Int8,Int16,Int32,Int64,Int128}) = base(10, x)

simonbyrne · 2018-01-24T19:29:45Z

I like that, especially since base isn't a verb, and it doesn't return the "base" of a number.

StefanKarpinski · 2018-01-24T19:55:39Z

Ok, let's go with that then! Delete all the verbs!!

bramtayl · 2018-01-25T03:09:54Z

Do we want to keep special hex printing for unsigned ints? In that case, the string method would have to be limited to Integers.

StefanKarpinski · 2018-01-25T05:35:22Z

Not a problem:

julia> string(0xff)
"255"

rfourquet · 2018-01-25T09:38:48Z

julia> string(0xff)
"255"

OT, but I still don't understand why it works this way, intead of of returning "0xff"...

JeffBezanson · 2018-01-25T18:47:08Z

Because we don't consider 0x to be a canonical number format. print and string of integers just gives digit sequences. show and repr of unsigned numbers includes the 0x.

StefanKarpinski · 2018-01-25T19:13:41Z

As it turns out, this was a good choice as it allows us to fold base into string now :)

simonbyrne · 2018-01-26T05:59:07Z

base/libgit2/oid.jl

 """
    raw(id::GitHash) -> Vector{UInt8}

 Obtain the raw bytes of the [`GitHash`](@ref) as a vector of length $OID_RAWSZ.
 """
 raw(id::GitHash) = collect(id.val)

-Base.string(id::AbstractGitHash) = hex(id)
+Base.string(id::GitHash) = join([string(i, base = 16, pad = 2) for i in id.val])
+Base.string(id::GitShortHash) = string(id.hash)[1:id.len]


I think the last discussion was to define Base.print(io::IO, id::GitHash) (which implicitly defines Base.string).

Oh no was it I'm so confused

e.g.

function Base.print(io::IO, id::GitHash) for i in id.val print(io, string(i, base = 16, pad = 2)) end end

Though this makes me think that we should also have the same args to print as well...

perhaps best to leave that for another PR though

bramtayl · 2018-01-26T06:04:28Z

slayage

StefanKarpinski · 2018-01-26T14:40:50Z

It would probably be good to get this passing tests before making any further changes. Otherwise you're just making it harder to figure out which of the changes is causing the build failure.

bramtayl · 2018-01-27T01:52:12Z

I just got through trying to build julia to help out with the debugging. These LoadError("sysimg.jl", are syntax errors?

rfourquet · 2018-01-27T02:49:27Z

Because we don't consider 0x to be a canonical number format.

Thanks for taking the time to answer! But this moves my question to why 0x is not considered a canonical number format... when I work with unsigned numbers, it's usually more meaningful for me to read them as 0x... (but I may lack imagination to see why it's more generally useful to print them as decimal numbers). Also, print(true) prints true, but it's hard to argue that true would be a canonical number format.

JeffBezanson · 2018-01-27T04:36:59Z

People want some way to convert a number to just a digit string, with no decoration. It's less clear what to do with booleans.

JeffBezanson · 2018-01-27T07:29:17Z

base/intfuncs.jl

@@ -549,16 +549,14 @@ julia> ndigits(12345)
 julia> ndigits(1022, 16)
 3

-julia> base(16, 1022)
+julia> string(1022, bass = 16)


https://www.youtube.com/watch?v=2IBqY0jWuZ8

rfourquet · 2018-01-27T08:46:12Z

People want some way to convert a number to just a digit string, with no decoration

dec ?

JeffBezanson · 2018-01-27T21:15:15Z

I don't understand the problem. If you want 0xff, use repr instead.

JeffBezanson · 2018-01-29T19:55:53Z

I'll try to rebase and fix this.

JeffBezanson · 2018-01-29T21:01:25Z

Rebased: #25804

bramtayl added 2 commits January 23, 2018 17:39

pad keyword for base

6246e1e

deprecate bin, oct, dec, hex

eb54a4e

bramtayl changed the title ~~Base keywords~~ Pad keyword for base, deprecate bin, oct, dec, hex Jan 24, 2018

JeffBezanson reviewed Jan 24, 2018

View reviewed changes

JeffBezanson requested changes Jan 24, 2018

View reviewed changes

JeffBezanson reviewed Jan 24, 2018

View reviewed changes

bramtayl added 5 commits January 24, 2018 12:03

restore history (phew)

a58e640

deprecate hex methods for githash

9b19270

deprecate char method for base

6da9e9d

removed bin, oct, dec, hex from exports

6721a4f

finish deprecating methods for git hash

5e26029

simonbyrne reviewed Jan 24, 2018

View reviewed changes

new methods for git string

76bb3e7

StefanKarpinski added triage This should be discussed on a triage call and removed triage This should be discussed on a triage call labels Jan 25, 2018

StefanKarpinski added this to the 1.0 milestone Jan 25, 2018

StefanKarpinski self-assigned this Jan 25, 2018

sooooo many tests

dd77ce2

simonbyrne reviewed Jan 26, 2018

View reviewed changes

misc cleanup

571fbf3

bramtayl changed the title ~~Pad keyword for base, deprecate bin, oct, dec, hex~~ Keywords for string, deprecate bin, oct, dec, hex, base Jan 26, 2018

JeffBezanson reviewed Jan 27, 2018

View reviewed changes

misc changes

bfe6f36

This was referenced Jan 29, 2018

API consistency review #20402

Closed

deprecate bin, oct, dec, hex, and base in favor of string and keyword args #25804

Merged

JeffBezanson closed this Jan 29, 2018

Keywords for string, deprecate bin, oct, dec, hex, base #25717

Keywords for string, deprecate bin, oct, dec, hex, base #25717

Conversation

bramtayl commented Jan 24, 2018

bramtayl commented Jan 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JeffBezanson left a comment

Choose a reason for hiding this comment

JeffBezanson Jan 24, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonbyrne commented Jan 24, 2018

JeffBezanson commented Jan 24, 2018

StefanKarpinski commented Jan 24, 2018

bramtayl commented Jan 24, 2018

simonbyrne commented Jan 24, 2018

StefanKarpinski commented Jan 24, 2018

bramtayl commented Jan 25, 2018 • edited Loading

StefanKarpinski commented Jan 25, 2018

rfourquet commented Jan 25, 2018

JeffBezanson commented Jan 25, 2018

StefanKarpinski commented Jan 25, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bramtayl commented Jan 26, 2018

StefanKarpinski commented Jan 26, 2018 • edited Loading

bramtayl commented Jan 27, 2018

rfourquet commented Jan 27, 2018

JeffBezanson commented Jan 27, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rfourquet commented Jan 27, 2018

JeffBezanson commented Jan 27, 2018

JeffBezanson commented Jan 29, 2018

JeffBezanson commented Jan 29, 2018

JeffBezanson Jan 24, 2018 •

edited

Loading

bramtayl commented Jan 25, 2018 •

edited

Loading

StefanKarpinski commented Jan 26, 2018 •

edited

Loading