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

proposal: math: add Clamp function #58146

Closed
gucio321 opened this issue Jan 30, 2023 · 13 comments
Closed

proposal: math: add Clamp function #58146

gucio321 opened this issue Jan 30, 2023 · 13 comments

Comments

@gucio321
Copy link

somethimes, I need to get a number clamped in a range. I think math package would be appropiate to add such a Clamp function.

Also, for some percentage calculation, Clamp01 method is useful as well.

(see #58144)

@gopherbot gopherbot added this to the Proposal milestone Jan 30, 2023
@seankhliao
Copy link
Member

What are the proposed function signatures?
Can you demonstrate why it should be in the standard library other than "it is sometimes useful"? ref https://go.dev/doc/faq#x_in_std

@ianlancetaylor ianlancetaylor changed the title proposal: math: ad Clamp function proposal: math: add Clamp function Jan 30, 2023
@gucio321
Copy link
Author

Can you demonstrate why it should be in the standard library other than "it is sometimes useful"?

Both of these functions, are parts of so called "standard libraries" in most of programming languages I know (C#, C++, rust).
ref: https://learn.microsoft.com/en-us/dotnet/api/system.math.clamp?view=net-7.0, https://en.cppreference.com/w/cpp/algorithm/clamp, https://docs.rs/num/0.2.1/num/fn.clamp.html

@earthboundkid
Copy link
Contributor

Clamp seems like a good idea, although I don't think math.Clamp01(v) is more clear than just math.Clamp(v, 0, 1). One question is whether this should wait for some kind of math/v2 with a generic Min, Max, etc.

Hmm, thinking out loud 🤔 :

  • Deprecate all the functions in math.
  • Add math/float and copy the existing math functions there. If it's practical to make them generic over float32/64 do it, but otherwise, just have them work with float64.
  • Add math/compare and put generic Min, Max, Clamp, and Compare(a, b) → -1,0,1 in it.

@gucio321
Copy link
Author

gucio321 commented Feb 1, 2023

I don't think math.Clamp01(v) is more clear than just math.Clamp(v, 0, 1).

It is quit useful if you do e.g. percentage operations or something like that. And Clamp01 VS Clamp(value, 0, 1) means less writing and if someone sees Clamp01 he already knows exactly the purpose of the statement. However I don't insist, I can limit this proposal to Clamp method only, if you think it would be better to leave Clamp01 for now.

One question is whether this should wait for some kind of math/v2 with a generic Min, Max, etc.

Personally, I don't think so. If current math api bases on float64s, we should leave it this way and any extensions should match this api for now. The other thing is that I find generics support in math library extremely useful, since making these conversions from float32 to 64 and back and again and so... makes using math annoying tbh.

Deprecate all the functions in math.
Add math/float and copy the existing math functions there. If it's practical to make them generic over float32/64 do it, but otherwise, just have them work with float64.

In my opinion deprecating current math is not really necessary.
If making it generic without breaking compatibility promise really IS NOT possible (it should be discussed tbh)
A subpackage (e.g. math/generic) should be created

@codenoid
Copy link

codenoid commented Mar 23, 2023

Rust has it

and, It's better to be math.Clamp only (wihout 01)

@gophun
Copy link

gophun commented Mar 23, 2023

@codenoid "Language X has it" is usually not a viable argument, otherwise Go would have to become a superset of all other languages.

@earthboundkid
Copy link
Contributor

I dunno, "language X has feature Y" isn't a definitive argument, but it is slight evidence for the usefulness of feature Y. A little bit of counter-evidence is that JavaScript has Math.max and Math.min, but not Math.clamp. I still lean towards math.Clamp being a good idea, but it should be part of a larger generic reorg, like #58559 (comment).

@rsc
Copy link
Contributor

rsc commented Mar 29, 2023

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Apr 5, 2023

math.Clamp(x, min, max float64) float64 seems okay although perhaps not above the bar.
Our current rule for the math package is that the bar is "what's in the C <math.h> library".
A generic math.Clamp that works on ints (or strings?)
would have to wait for a math/v2 or some other answer to generics in std.

math.Clamp01 seems too special-purpose either way.

@rsc
Copy link
Contributor

rsc commented Apr 12, 2023

Based on the discussion above, this proposal seems like a likely decline.
— rsc for the proposal review group

@gucio321
Copy link
Author

Based on the discussion above, this proposal seems like a likely decline. — rsc for the proposal review group

@rsc according to the discussion above, it seems that Clamp is a wanted feature. Could you please justify the refusal?

@ianlancetaylor
Copy link
Contributor

@gucio321 Nobody would propose a feature that nobody wants. The question is not: does anybody want this feature? If we followed that guideline, we would add every feature and the result would be an unmanageable and incomprehensible mash of features. The question is: is this feature worth the additional complexity?

As @rsc said above, we have a simple guideline today for the math package: anything that is in the C <math.h> library. If we're going to add math.Clamp, we need a different guideline. What should that be? It shouldn't be "anything that people will find useful."

Thanks.

@rsc
Copy link
Contributor

rsc commented Apr 19, 2023

No change in consensus, so declined.
— rsc for the proposal review group

@rsc rsc closed this as completed Apr 19, 2023
@golang golang locked and limited conversation to collaborators Apr 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants