dysonance / Strategems.jl

Quantitative systematic trading strategy development and backtesting in Julia
Other
163 stars 39 forks source link

Incompatible with Julia 1.0v #17

Closed gruberdev closed 5 years ago

gruberdev commented 5 years ago

When installing the package, you're met with the following message:

ERROR: Unsatisfiable requirements detected for package Strategems [054b7d4e]:
 Strategems [054b7d4e] log:
 ├─possible versions are: 0.0.1-0.0.5 or uninstalled
 ├─restricted to versions * by an explicit requirement, leaving only versions 0.0.1-0.0.5
 └─restricted by julia compatibility requirements to versions: uninstalled — no versions left

The whole Git seems to have been deprecated, is the developer still active?

femtotrader commented 5 years ago

A good way to know if a developer is alive (ie still active) is by submitting him pull request. Somes examples:

gruberdev commented 5 years ago

A good way to know if a developer is alive (ie still active) is by submitting him pull request. Somes examples:

I cloned the repository and tested the code on the 0.6.4v, yet, the code seems to be incompatible due to Indicator's being 0.6.0v only (That's what I took from the error I received), and there's no Windows binary for the older 0.6.0v release. So now I'm compilling that specific version to run it properly and port it to 1.0.0v as a pull request eventually.

femtotrader commented 5 years ago

https://github.com/dysonance/Strategems.jl/blob/master/src/rule.jl#L15-L21

doesn't work with Julia 0.7.

It raises

ERROR: syntax: "..." expression outside call
gruberdev commented 5 years ago

https://github.com/dysonance/Strategems.jl/blob/master/src/rule.jl#L15-L21

doesn't work with Julia 0.7.

It raises

ERROR: syntax: "..." expression outside call

This seems similar to: https://github.com/JuliaLang/julia/issues/23218 Where building a tuple inside a tuple literal happens to raise an error on 0.7.0v. Although I don't have any meaningful fixes for that, did you fix that on your new branch? If you didn't, could you make an specific issue for it?

femtotrader commented 5 years ago

Not sure if that's a correct fix or not:

https://github.com/dysonance/Strategems.jl/pull/19/files#diff-a7f10850a9e8c4fb546b622a1813a813L19

femtotrader commented 5 years ago

Tests are passing with Julia 0.7 so:

gruberdev commented 5 years ago

Not sure if that's a correct fix or not:

https://github.com/dysonance/Strategems.jl/pull/19/files#diff-a7f10850a9e8c4fb546b622a1813a813L19

I think that's the correct fix. The splat there is forcefully converting the variable to a possible array, whereas 0.7.0v does this conversion automatically when needed. This should result in a conflict that is resolved when you remove the (...), the question is, why that didn't happen with the other splats? Maybe they all were used as arrays and that specific variable didn't?

Edit: I investigated further, and found that 0.7.0v supports direct specification of splatting interpolation: https://docs.julialang.org/en/v0.7/manual/metaprogramming/#Splatting-interpolation-1 Where I can only find proper "..." usage on 0.6.0v here: https://docs.julialang.org/en/v0.6.0/manual/faq/#The-two-uses-of-the-...-operator:-slurping-and-splatting-1 Which the splatting operator only works in the function definition as an operator to "soak a list", as classical programming and other languages do. I am confused if your fix works as the original intended to, we'll see. Good to see the build is passing, I'm going to try your changes to the code and report back.

dysonance commented 5 years ago

@VerifiedGruber @femtotrader Thanks for bringing this to light guys, looking into it now. Will review the PR changes and try to figure out what's required to get this working again.

Also, my apologies for the lag in getting this package up to date with newer Julia versions.

dysonance commented 5 years ago

@VerifiedGruber @femtotrader Hey guys, just merged the above request to get this package working for current Julia versions. I've released a new version for these updates (v0.1.0) and opened the pull request on Julia's METADATA repository to get this work pushed out.

Closing this issue for now, let me know if you guys experience any more issues and we can continue making improvements. Thanks again for raising this issue, and thanks to @femtotrader for all the leg work he did to minimize the final steps required to get this done. Cheers guys.