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

Compat hasmethod for older Julia? #18

Closed
aminya opened this issue May 18, 2020 · 3 comments · Fixed by #20
Closed

Compat hasmethod for older Julia? #18

aminya opened this issue May 18, 2020 · 3 comments · Fixed by #20

Comments

@aminya
Copy link
Contributor

aminya commented May 18, 2020

If I want to use Tricks in my package, I should do a lot of hackeries to only load Tricks if VERSION is above 1.3 and then do something like following to make sure that this works across all the versions.

Isn't it better to relax the compat bound of Julia in Tricks and define the following for compatibility?

@static if VERSION < v"1.3"
    static_hasmethod(args...) = hasmethod(args...)
end

This way I can just use Tricks without any concerns.

@oxinabox
Copy link
Owner

I don't think I agree.
because the difference is very significant.
It is between zero run time cost and fairly substantial run-time cost

Potentially a method could be added:

 @static if VERSION < v"1.3"
    const compat_hasmethod = hasmethod
else
    const compat_hasmethod = static_hasmethod
end

then one can use compat_hasmethod knowing that it varies depending on release.
and we could relax the constraints in this package.

But i think making it called static_hasmethod when it might not be static is probably not a good thing to do.

@aminya
Copy link
Contributor Author

aminya commented May 19, 2020

Yes, the name of the function is not an issue. That compat_hasmethod is a good idea!

@aminya
Copy link
Contributor Author

aminya commented Jun 14, 2020

@oxinabox Any update on this? My aminya/AcuteML.jl#135 is waiting for this 😁

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

Successfully merging a pull request may close this issue.

2 participants