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

Fix testing failure on julia 1.0 #222

Closed
wants to merge 3 commits into from

Conversation

felixcremer
Copy link
Contributor

This removes the import of LinearAlgebra: gradient and exports the gradient function because it has been fully removed (see JuliaLang/julia#16113).
It also includes the changes of #221 to Random.srand.

The gradient function was completely removed and not moved to LinearAlgebra as documented in the depwarn.
This import fails therefore.
This is needed, because we don't import the gradient function anymore.
@tomasaschan
Copy link
Contributor

Gradient seems to me like it's a too general concept to be "owned" by Interpolations.jl. Is there somewhere else we should import the name from to make sure we're extending the concept, rather than exporting a new one? (Or is import merging solved nowadays?)

@timholy
Copy link
Member

timholy commented Aug 11, 2018

No, import merging isn't solved. I don't think we should export it.

Copy link
Contributor

@tomasaschan tomasaschan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since #221 is merged now, this needs rebasing anyway, and the changes to test/scaling/nointerp.jl in this PR can be discarded.

Where has LinearAlgebra.gradient moved? Could gradient be imported from the new location?

If not, I vote for completely removing the export of gradient (and, possibly, gradient!, gradient1 and the corresponding /hessian[!1]?/ functions, for consistency), and promoting qualified use of those methods instead. (This would have to go through a deprecation cycle, of course.)

@mkborregaard
Copy link

Is it correct that merge of this PR is what keeps this package back from 1.0 compatibility?

@tomasaschan
Copy link
Contributor

@mkborregaard Technically, no. But (one of) the problem(s) addressed by this PR is, namely that we need to decide how to handle that gradient is no longer exported from base.

@felixcremer
Copy link
Contributor Author

Everything this tries to solve is handled in #226

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 this pull request may close these issues.

4 participants