-
Notifications
You must be signed in to change notification settings - Fork 69
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
Julia 0.7 #370
Julia 0.7 #370
Conversation
Codecov Report
@@ Coverage Diff @@
## master #370 +/- ##
===========================================
+ Coverage 86.86% 99.13% +12.27%
===========================================
Files 10 9 -1
Lines 510 346 -164
===========================================
- Hits 443 343 -100
+ Misses 67 3 -64
Continue to review full report at Codecov.
|
.travis.yml
Outdated
env: | ||
- PYTHON="" | ||
notifications: | ||
email: false | ||
sudo: false | ||
script: | ||
- if [[ -a .git/shallow ]]; then git fetch --unshallow; fi | ||
- julia -e 'Pkg.clone(pwd()); Pkg.build("TimeSeries"); Pkg.test("TimeSeries", coverage=true)' | ||
- julia -e 'Pkg.clone(pwd()); Pkg.build("TimeSeries")' | ||
- julia -e 'Pkg.test("TimeSeries", coverage=true)' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For compatibility with 0.7/1.0, you should just remove or comment out the script
block entirely. That allows Travis to use its default script for Julia, which is set up to work with all existing versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, okay.
I think TimeSeries.jl/test/runtests.jl Line 13 in b0773e8
should be removed according https://travis-ci.org/JuliaStats/TimeSeries.jl/builds/426145131#L673 |
include("split.jl") | ||
include("apply.jl") | ||
include("broadcast.jl") | ||
include("combine.jl") | ||
include("readwrite.jl") | ||
include("utilities.jl") | ||
include("modify.jl") | ||
include("basemisc.jl") | ||
include("deprecated.jl") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why keeping an empty deprecated.jl
file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to keep src/deprecated.jl
for future use, but I do not want to test the deprecation binding in the future, since it mess up the testing output. I just removed the test/deprecated.jl
This PR is ready for review. |
LGTM... tests are passing both with Julia 0.7 and 1.0 |
|
||
abstract type AbstractTimeSeries end | ||
abstract type AbstractTimeSeries{T,N,D} end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A notable change: I make this abstract type being parametric, to help some iteration stuffs like eltype
.
TODO:
https://docs.julialang.org/en/v1/manual/interfaces/#man-interfaces-broadcasting-1
Base.findall
support