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.

Sign up for GitHub

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

Jump to bottom

deprecate/remove gradient from base #23816

Merged
merged 1 commit into from
Sep 25, 2017
Merged

deprecate/remove gradient from base #23816

merged 1 commit into from
Sep 25, 2017

Conversation

fredrikekre
Copy link
Member

No description provided.

@fredrikekre fredrikekre added kind:deprecation This change introduces or involves a deprecation domain:linear algebra Linear algebra labels Sep 22, 2017
@fredrikekre fredrikekre mentioned this pull request Sep 22, 2017
Closed
@fredrikekre fredrikekre added this to the 1.0 milestone Sep 22, 2017
@fredrikekre fredrikekre added the status:triage This should be discussed on a triage call label Sep 22, 2017
ararslan
ararslan reviewed Sep 22, 2017
View reviewed changes
NEWS.md
@@ -465,6 +465,8 @@ Deprecated or removed
`prompt_if_incorrect` argument are deprecated. Instead, prompting behavior is controlled using
the `allow_prompt` keyword in the `LibGit2.CredentialPayload` constructor ([#23690]).

* `gradient` is deprecated and will be removed in the next release ([#23816]).
Copy link
Member

Choose a reason for hiding this comment

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

I think "will be removed in the next release" is redundant with "deprecated," since we always remove deprecated things in the subsequent release. Probably doesn't matter too much though.

Copy link
Member Author

Choose a reason for hiding this comment

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

Usually deprecation messages suggests a replacement; I thought it would be nice to mention that there won't be one.

@fredrikekre
Copy link
Member Author

#23796 on Travis x86_64

@JeffBezanson
Copy link
Sponsor Member

Ready to merge?

@ararslan
Copy link
Member

Still has the [triage] label. Does it still need to be triaged?

@StefanKarpinski
Copy link
Sponsor Member

Does it still need to be triaged?

No, we should just do this.

@StefanKarpinski StefanKarpinski removed the status:triage This should be discussed on a triage call label Sep 25, 2017
@fredrikekre fredrikekre merged commit 79bae3f into master Sep 25, 2017
@fredrikekre fredrikekre deleted the fe/gradient branch September 25, 2017 18:36
@cormullion cormullion mentioned this pull request Mar 18, 2018
Closed
@tomasaschan
Copy link
Member

I'm not saying this change wasn't good, but it has broken Interpolations.jl in a way that seems a little tricky to figure out how to solve; see e.g. JuliaMath/Interpolations.jl#222.

Because we previously imported gradient and extended it with new methods, Interpolations.jl doesn't work on 1.0, where there is no longer a gradient function to import. However, exporting gradient from Interpolations.jl also feels wrong; it's not a concept inherently related to interpolation, so it shouldn't be "owned" by that package.

Is there a recommended place/package from which to import gradient now that it's not in base anymore? Is there some general advice for how to handle this situation?

@andreasnoack
Copy link
Member

For now, I'd just define a non-exported gradient function in Interpolations.

@joseraulgonzalez joseraulgonzalez mentioned this pull request Aug 15, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andreasnoack andreasnoack andreasnoack approved these changes

@ararslan ararslan ararslan approved these changes

Assignees
No one assigned
Labels
domain:linear algebra Linear algebra kind:deprecation This change introduces or involves a deprecation
Projects
None yet
Milestone
0.7
Development

Successfully merging this pull request may close these issues.
6 participants
@fredrikekre @JeffBezanson @ararslan @StefanKarpinski @tomasaschan @andreasnoack