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.

Sign up for GitHub

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

Jump to bottom

Add Compat.Unicode for standard library Unicode #432

Merged
merged 4 commits into from
Dec 21, 2017
Merged

Add Compat.Unicode for standard library Unicode #432

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6847401
Add Compat.Unicode for standard library Unicode
omus Dec 15, 2017
51aa82f
Define titlecase only on Julia 0.5
omus Dec 18, 2017
2b404a4
Remove normalize and isassigned from exports
omus Dec 18, 2017
56fe7f9
Add textwidth deprecation
omus Dec 18, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,12 @@ Currently, the `@compat` macro supports the following syntaxes:
`CartesianRange` now has two type parameters, so using them as
fields in other `struct`s requires manual intervention.

## Module Aliases

* In 0.6, some 0.5 iterator functions have been moved to the `Base.Iterators`
module. Code can be written to work on both 0.5 and 0.6 by `import`ing or
`using` the `Compat.Iterators` module instead. ([#18839])

* `using Compat.Test`, `using Compat.SharedArrays`, `using Compat.Mmap`, and `using
Compat.DelimitedFiles` are provided on versions older than 0.7, where these are not yet
part of the standard library. ([#23931])
Expand All @@ -89,11 +95,8 @@ Currently, the `@compat` macro supports the following syntaxes:
* `using Compat.Dates` is provided on versions older than 0.7, where this library is not
yet a part of the standard library. ([#24459])

## Module Aliases

* In 0.6, some 0.5 iterator functions have been moved to the `Base.Iterators`
module. Code can be written to work on both 0.5 and 0.6 by `import`ing or
`using` the `Compat.Iterators` module instead. ([#18839])
* `using Compat.Unicode` is provided on versions older than 0.7, where this library is not
yet a part of the standard library. ([#25021])
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 diff is messed up here. I moved all of the using X notes into the "Module Aliases" section.


## New functions, macros, and methods

Expand Down Expand Up @@ -236,8 +239,6 @@ Currently, the `@compat` macro supports the following syntaxes:

* `IntSet` is now `BitSet` ([#24282])

* `strwidth` and `charwidth` are now merged into `textwidth` ([#23667]).

* `Complex32`, `Complex64`, and `Complex128` are now `ComplexF16`, `ComplexF32`, and
`ComplexF64`, respectively ([#24647]).

Expand Down Expand Up @@ -375,7 +376,6 @@ includes this fix. Find the minimum version from there.
[#23427]: https://github.com/JuliaLang/julia/issues/23427
[#23570]: https://github.com/JuliaLang/julia/issues/23570
[#23666]: https://github.com/JuliaLang/julia/issues/23666
[#23667]: https://github.com/JuliaLang/julia/issues/23667
[#23757]: https://github.com/JuliaLang/julia/issues/23757
[#23812]: https://github.com/JuliaLang/julia/issues/23812
[#23931]: https://github.com/JuliaLang/julia/issues/23931
Expand All @@ -389,3 +389,4 @@ includes this fix. Find the minimum version from there.
[#24652]: https://github.com/JuliaLang/julia/issues/24652
[#24657]: https://github.com/JuliaLang/julia/issues/24657
[#24785]: https://github.com/JuliaLang/julia/issues/24785
[#25021]: https://github.com/JuliaLang/julia/issues/25021
48 changes: 41 additions & 7 deletions src/Compat.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -752,13 +752,6 @@ end
export BitSet
end

# 0.7.0-DEV.1930
@static if !isdefined(Base, :textwidth)
textwidth(c::Char) = charwidth(c)
textwidth(c::AbstractString) = strwidth(c)
export textwidth
end

Copy link
Member Author

@omus omus Dec 15, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure what the procedure with deprecating a Compat supported change is. Since the rule for Compat.jl is to use the latest syntax it seemed reasonable to move textwidth into the Unicode module. Do I need to deprecate this method so that it's not breaking?

Copy link
Member

Choose a reason for hiding this comment

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

I would prefer a deprecation over a breaking change. It is quite common for packages to require a fairly recent Compat version, so if others need to set an upper bound, that could be rather unfortunate.

But I have no idea want the standard procedure is.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added a deprecation for this so the transition should be non-breaking.

# 0.7.0-DEV.2116
@static if VERSION < v"0.7.0-DEV.2116"
import Base: spdiagm
Expand Down Expand Up @@ -874,6 +867,47 @@ end
export ComplexF64
end

# 0.7.0-DEV.2915
module Unicode
export graphemes, textwidth, isvalid,
islower, isupper, isalpha, isdigit, isxdigit, isnumeric, isalnum,
iscntrl, ispunct, isspace, isprint, isgraph,
lowercase, uppercase, titlecase, lcfirst, ucfirst

if VERSION < v"0.7.0-DEV.2915"
# 0.7.0-DEV.1930
if !isdefined(Base, :textwidth)
textwidth(c::Char) = charwidth(c)
textwidth(c::AbstractString) = strwidth(c)
end

isnumeric(c::Char) = isnumber(c)

# 0.6.0-dev.1404 (https://github.com/JuliaLang/julia/pull/19469)
if !isdefined(Base, :titlecase)
titlecase(c::Char) = isascii(c) ? ('a' <= c <= 'z' ? c - 0x20 : c) :
Char(ccall(:utf8proc_totitle, UInt32, (UInt32,), c))

function titlecase(s::AbstractString)
startword = true
b = IOBuffer()
for c in s
if isspace(c)
print(b, c)
startword = true
else
print(b, startword ? titlecase(c) : c)
startword = false
end
end
return String(take!(b))
end
end
else
using Unicode
end
end

include("deprecated.jl")

end # module Compat
4 changes: 4 additions & 0 deletions src/deprecated.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,3 +188,7 @@ else
import Base.@irrational
import Base.LinAlg.BLAS.@blasfunc
end

if VERSION < v"0.7.0-DEV.2915"
Base.@deprecate textwidth Compat.Unicode.textwidth
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand the justification for these lines. They are annoying as they print deprecation warnings on Julia 0.6. These can appear for packages which worked perfectly well before, just because Compat has been updated.

Copy link
Member Author

Choose a reason for hiding this comment

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

Since textwidth needed to be moved inside Compat.Unicode module the only two real choices were to either make a alias at the top-level or to add a deprecation. I ended up going for the deprecation since Compat is about supporting "the latest syntax in a backwards-compatible way".

Copy link
Member Author

Choose a reason for hiding this comment

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

I personally haven't seen this scenario where previous Compat code was required to be moved. There may be a standard procedure on how to handle this for which I am unaware.

Copy link
Member

Choose a reason for hiding this comment

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

OK, I see the logic. However I think it's bad to generate tons of deprecation warnings on Julia 0.6 for packages which worked perfectly fine until now. For example, DataFrames is very painful to use right now because of this.

I think the best approach is to keep exporting this function on Julia < 0.7 since it's supported by Julia 0.6, but not on Julia 0.7. We don't need to deprecate the function ourselves since Julia takes care of that already. See #437.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good to me. We should think about making a CONTRIBUTING.md to Compat.jl to clarify these kind of details.

end
22 changes: 17 additions & 5 deletions test/runtests.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,10 +320,10 @@ let s = "Koala test: 🐨"
end

# julia#17155, tests from Base Julia
@test (uppercase∘hex)(239487) == "3A77F"
@test (Compat.Unicode.uppercase∘hex)(239487) == "3A77F"
let str = randstring(20)
@test filter(!isupper, str) == replace(str, r"[A-Z]", "")
@test filter(!islower, str) == replace(str, r"[a-z]", "")
@test filter(!Compat.Unicode.isupper, str) == replace(str, r"[A-Z]", "")
@test filter(!Compat.Unicode.islower, str) == replace(str, r"[a-z]", "")
end

# julia#19950, tests from Base (#20028)
Expand Down Expand Up @@ -891,8 +891,8 @@ end
@test 1 in BitSet(1:10)

# 0.7.0-DEV.1930
@test textwidth("A") == 1
@test textwidth('A') == 1
@test Compat.Unicode.textwidth("A") == 1
@test Compat.Unicode.textwidth('A') == 1

# 0.7
@test diagm(0 => ones(2), -1 => ones(2)) == [1.0 0.0 0.0; 1.0 1.0 0.0; 0.0 1.0 0.0]
Expand Down Expand Up @@ -957,6 +957,18 @@ end
@test ComplexF32 === Complex{Float32}
@test ComplexF64 === Complex{Float64}

# 0.7.0-DEV.2915
module Test25021
using Compat
using Compat.Test
using Compat.Unicode
@test isdefined(@__MODULE__, :Unicode)

@test !isnumeric('a')
@test isnumeric('1')
@test titlecase("firstname lastname") == "Firstname Lastname"
end

if VERSION < v"0.6.0"
include("deprecated.jl")
end
Expand Down