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.

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

Incompatible with Julia 1.0v #17

Closed
gruberdev opened this issue Oct 3, 2018 · 9 comments
Closed

Incompatible with Julia 1.0v #17

gruberdev opened this issue Oct 3, 2018 · 9 comments

Comments

@gruberdev
Copy link

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
Copy link
Collaborator

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

@gruberdev
Copy link
Author

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
Copy link
Collaborator

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
Copy link
Author

gruberdev commented Oct 4, 2018

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:
JuliaLang/julia#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
Copy link
Collaborator

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

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

@femtotrader
Copy link
Collaborator

Tests are passing with Julia 0.7 so:

  • maybe that's fixed or
  • maybe test coverage is not enough

@gruberdev
Copy link
Author

gruberdev commented Oct 4, 2018

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
Copy link
Owner

@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
Copy link
Owner

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants