Fix compatibility with Julia 0.7 #30
Conversation
ararslan
changed the title
ararslan
force-pushed
the
aa/0.7
branch
2 times, most recently
from
5d6dedb
to
addc82e
Compare
|
This is happening on 0.7, not sure what to make of it yet: |
|
AHA! It's JuliaLang/julia#24195. Indeed: We should not be overloading |
Codecov Report
@@ Coverage Diff @@
## master #30 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 5 5
Lines 184 179 -5
=====================================
- Hits 184 179 -5
Continue to review full report at Codecov.
|
|
This is an odd error that only happens on 0.7: |
|
|
|
Tests pass on 0.7 with JuliaTime/TimeZones.jl#137. There are still a few deprecation warnings to tackle though. |
REQUIRE
Outdated
| @@ -1,3 +1,3 @@ | |||
| julia 0.6 | |||
| TimeZones 0.4 | |||
|
|
||
| function Base.show(io::IO, ::Type{AnchoredInterval{P, T}}) where {P, T} | ||
| print(io, "AnchoredInterval{$P, $T}") | ||
| end |
There was a problem hiding this comment.
I think the intention was to avoid these being fully qualified on Julia 0.6. Should we just have a version check around this to limit it's usage to Julia 0.6?
There was a problem hiding this comment.
The problem is that it's always a Bad Thing To DoTM, it's just penalized with actual crashing on 0.7.
There was a problem hiding this comment.
Why is it always a bad thing to do? I'm mostly just curious so I can give some rational against doing this.
There was a problem hiding this comment.
See #31, in particular the link to Jeff's comment. Basically it's redefining what it means to call show on a Type.
test/runtests.jl
Outdated
| compactstring(x) = sprint(show, x, context=:compact=>true) | ||
| else | ||
| compactstring(x) = sprint(showcompact, x) | ||
| end |
There was a problem hiding this comment.
Although this won't work generally in Compat we can use this here to use the new 0.7 sprint syntax instead of making a new function:
if VERSION < v"0.7.0-DEV.4524"
function sprint(f::Function, args...; context=nothing)
if context !== nothing
Base.sprint((io, args...) -> f(IOContext(io, context), args...), args...)
else
Base.sprint(f, args...)
end
end
endThen we can just use sprint(show, x, context=:compact=>true) in the tests.
In particular: * Use definitions from Compat, e.g. Compat.Dates * Define a local version of sprint(show, args; kwargs), as this is not available in Compat * Use arguments in DomainError constructors * Remove custom show methods for types * Raise the minimum required version of TimeZones
|
Tests are passing on 0.7 now. 🙂 There are still a few more deprecations, in particular |
| @@ -5,6 +5,7 @@ os: | |||
| - osx | |||
| julia: | |||
| - 0.6 | |||
| - 0.7 | |||
There was a problem hiding this comment.
We may also want to add 0.7 on AppVeyor
|
I think we should go ahead and merge this once CI finishes so that the package works on 0.7, then the remaining deprecations can be addressed separately. |
ararslan
changed the title
|
@ararslan I'm ok with merging as is. Looks like AppVeyor is failing from a TimeZones.jl compatibility issue. Do you want to wait for TimeZones to be fixed or just merge this? |
|
Hopefully will be addressed with: JuliaTime/TimeZones.jl#139 |
I'll leave that up to you. |
|
The TimeZones issue more trouble than I originally thought. I won't hold this up any longer. I'll tag a new version tomorrow. |
This change allows the package to load on 0.7 and also fixes all deprecations emitted on 0.7.
...once it's done