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 clamp! #22247

Closed
wants to merge 4 commits into from
Closed

Deprecate clamp! #22247

wants to merge 4 commits into from

Conversation

arghhhh
Copy link
Contributor

@arghhhh arghhhh commented Jun 6, 2017 •
edited by ararslan
Loading

Fixes #22236

@fredrikekre
Copy link
Member

Should be removed here too:

julia/base/exports.jl

Line 445 in 80369bf

clamp!,

julia/base/math.jl

Line 15 in 80369bf

clamp, clamp!, modf, ^, mod2pi, rem2pi,

@ararslan ararslan added arrays [a, r, r, a, y, s] deprecation This change introduces or involves a deprecation labels Jun 6, 2017
@arghhhh
Copy link
Contributor Author

arghhhh commented Jun 6, 2017

There's a reason for my github name! OK. I have removed these two mentions of clamp!, and grep'ed the rest of source code. There is an unrelated clamp! in base/grisu/bignums.

tkelman
tkelman reviewed Jun 7, 2017
View reviewed changes
base/deprecated.jl Outdated
@@ -748,6 +748,7 @@ end

# Deprecate manually vectorized clamp methods in favor of compact broadcast syntax
@deprecate clamp(A::AbstractArray, lo, hi) clamp.(A, lo, hi)
@deprecate clamp!(A::AbstractArray, lo, hi) A .= clamp.(A, lo, hi)
Copy link
Contributor

Choose a reason for hiding this comment

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

this needs to be in the 0.7 section down near the end of the file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. I appreciate your patience with me. Thanks.

@tkelman
Copy link
Contributor

tkelman commented Jun 9, 2017

doc build is having trouble on CI, looks like you need to remove doc/src/stdlib/math.md line 134

@JeffBezanson JeffBezanson added the triage This should be discussed on a triage call label Jan 15, 2018
@JeffBezanson
Copy link
Sponsor Member

This function is actually not a huge problem --- since clamp! only has a method for arrays, it can't be confused with a scalar function generalized to vectors.

@StefanKarpinski
Copy link
Sponsor Member

I agree. I find clamp!(A, lo, hi) quite convenient and replacing it with A .= clamp(A, lo, hi) while doable seems kind of unnecessary.

@StefanKarpinski StefanKarpinski removed the triage This should be discussed on a triage call label Jan 18, 2018
@fredrikekre fredrikekre mentioned this pull request Jul 5, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tkelman tkelman tkelman left review comments

Assignees
No one assigned
Labels
arrays [a, r, r, a, y, s] deprecation This change introduces or involves a deprecation
Projects
None yet
Milestone
No milestone
Development

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