ruby-core@ruby-lang.org archive (unofficial mirror)
 help / color / mirror / Atom feed
* [ruby-core:105377] [Ruby master Bug#18187] Float#clamp() returns ArgumentError (comparison of Float with 1 failed)
@ 2021-09-22 17:23 SouravGoswami (Sourav Goswami)
  2021-09-22 21:04 ` [ruby-core:105380] " mame (Yusuke Endoh)
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: SouravGoswami (Sourav Goswami) @ 2021-09-22 17:23 UTC (permalink / raw)
  To: ruby-core

Issue #18187 has been reported by SouravGoswami (Sourav Goswami).

----------------------------------------
Bug #18187: Float#clamp() returns ArgumentError (comparison of Float with 1 failed)
https://bugs.ruby-lang.org/issues/18187

* Author: SouravGoswami (Sourav Goswami)
* Status: Open
* Priority: Normal
* ruby -v: ruby 3.0.2p107 (2021-07-07 revision 0db68f0233) [x86_64-linux]
* Backport: 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN
----------------------------------------
When I have a Float::NAN as a number, I expect all the method to work properly.

For example, `Float::NAN - 1` gives NAN. But Float::NAN.to_i raises FloatDomainError.

But in case of clamp(), Float::NAN.clamp(0, 100) returns `ArgumentError (comparison of Float with 1 failed)`

This error doesn't explain what's actually wrong. I didn't write the comparison to compare Float with 1. I didn't pass any invalid argument either. This error is a reflection of what's going on in the C level, which shouldn't appear to the user.

If I write a vanilla clamp() in ruby:

```
Float.define_method(:clamp2) { |min, max| self < min ? min : self > max ? max : self }
```

In this case, I can call it like this:

```
> 8.0.clamp2(10, 100)
=> 10
> 80.0.clamp2(10, 100)
=> 80.0
> 800.0.clamp2(10, 100)
=> 100
> Float::NAN.clamp2(10, 100)
=> NaN
```

As you can see, it just returns NAN. But in case of the built-in clamp, it raises the ArgumentError, even though my arguments are just correct. So this should handle this clamp() correctly, either returning the min value or `Float::NAN`.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [ruby-core:105380] [Ruby master Bug#18187] Float#clamp() returns ArgumentError (comparison of Float with 1 failed)
  2021-09-22 17:23 [ruby-core:105377] [Ruby master Bug#18187] Float#clamp() returns ArgumentError (comparison of Float with 1 failed) SouravGoswami (Sourav Goswami)
@ 2021-09-22 21:04 ` mame (Yusuke Endoh)
  2021-09-22 21:30 ` [ruby-core:105382] " SouravGoswami (Sourav Goswami)
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: mame (Yusuke Endoh) @ 2021-09-22 21:04 UTC (permalink / raw)
  To: ruby-core

Issue #18187 has been updated by mame (Yusuke Endoh).


On my machine, the code raises `comparison of Float with 0 failed`, instead of `... with 1 failed`. I have no idea where `1` comes from.

```
$ ruby -ve 'Float::NAN.clamp(0, 100)'
ruby 3.0.2p107 (2021-07-07 revision 0db68f0233) [x86_64-linux]
-e:1:in `clamp': comparison of Float with 0 failed (ArgumentError)
        from -e:1:in `<main>'
```

BTW, I have no opinion about what `Float::NAN.clamp(0, 100)` should return or raise.

----------------------------------------
Bug #18187: Float#clamp() returns ArgumentError (comparison of Float with 1 failed)
https://bugs.ruby-lang.org/issues/18187#change-93794

* Author: SouravGoswami (Sourav Goswami)
* Status: Open
* Priority: Normal
* ruby -v: ruby 3.0.2p107 (2021-07-07 revision 0db68f0233) [x86_64-linux]
* Backport: 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN
----------------------------------------
When I have a Float::NAN as a number, I expect all the method to work properly.

For example, `Float::NAN - 1` gives NAN. But Float::NAN.to_i raises FloatDomainError.

But in case of clamp(), Float::NAN.clamp(0, 100) returns `ArgumentError (comparison of Float with 1 failed)`

This error doesn't explain what's actually wrong. I didn't write the comparison to compare Float with 1. I didn't pass any invalid argument either. This error is a reflection of what's going on in the C level, which shouldn't appear to the user.

If I write a vanilla clamp() in ruby:

```
Float.define_method(:clamp2) { |min, max| self < min ? min : self > max ? max : self }
```

In this case, I can call it like this:

```
> 8.0.clamp2(10, 100)
=> 10
> 80.0.clamp2(10, 100)
=> 80.0
> 800.0.clamp2(10, 100)
=> 100
> Float::NAN.clamp2(10, 100)
=> NaN
```

As you can see, it just returns NAN. But in case of the built-in clamp, it raises the ArgumentError, even though my arguments are just correct. So this should handle this clamp() correctly, either returning the min value or `Float::NAN`.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [ruby-core:105382] [Ruby master Bug#18187] Float#clamp() returns ArgumentError (comparison of Float with 1 failed)
  2021-09-22 17:23 [ruby-core:105377] [Ruby master Bug#18187] Float#clamp() returns ArgumentError (comparison of Float with 1 failed) SouravGoswami (Sourav Goswami)
  2021-09-22 21:04 ` [ruby-core:105380] " mame (Yusuke Endoh)
@ 2021-09-22 21:30 ` SouravGoswami (Sourav Goswami)
  2021-09-23 17:55 ` [ruby-core:105398] " jeremyevans0 (Jeremy Evans)
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: SouravGoswami (Sourav Goswami) @ 2021-09-22 21:30 UTC (permalink / raw)
  To: ruby-core

Issue #18187 has been updated by SouravGoswami (Sourav Goswami).


Hi, sorry, yes it's `comparison of Float with 0 failed`, probably there was some typo.

I agree it should raise or return. But it shouldn't raise ArgumentError. Anyway, it probably has the lowest priority because it doesn't cause any issue so far.

The best would be returning `Float::NAN`? Because that's the behaviour you get when you write comparison in Ruby.

----------------------------------------
Bug #18187: Float#clamp() returns ArgumentError (comparison of Float with 1 failed)
https://bugs.ruby-lang.org/issues/18187#change-93796

* Author: SouravGoswami (Sourav Goswami)
* Status: Open
* Priority: Normal
* ruby -v: ruby 3.0.2p107 (2021-07-07 revision 0db68f0233) [x86_64-linux]
* Backport: 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN
----------------------------------------
When I have a Float::NAN as a number, I expect all the method to work properly.

For example, `Float::NAN - 1` gives NAN. But Float::NAN.to_i raises FloatDomainError.

But in case of clamp(), Float::NAN.clamp(0, 100) returns `ArgumentError (comparison of Float with 1 failed)`

This error doesn't explain what's actually wrong. I didn't write the comparison to compare Float with 1. I didn't pass any invalid argument either. This error is a reflection of what's going on in the C level, which shouldn't appear to the user.

If I write a vanilla clamp() in ruby:

```
Float.define_method(:clamp2) { |min, max| self < min ? min : self > max ? max : self }
```

In this case, I can call it like this:

```
> 8.0.clamp2(10, 100)
=> 10
> 80.0.clamp2(10, 100)
=> 80.0
> 800.0.clamp2(10, 100)
=> 100
> Float::NAN.clamp2(10, 100)
=> NaN
```

As you can see, it just returns NAN. But in case of the built-in clamp, it raises the ArgumentError, even though my arguments are just correct. So this should handle this clamp() correctly, either returning the min value or `Float::NAN`.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [ruby-core:105398] [Ruby master Bug#18187] Float#clamp() returns ArgumentError (comparison of Float with 1 failed)
  2021-09-22 17:23 [ruby-core:105377] [Ruby master Bug#18187] Float#clamp() returns ArgumentError (comparison of Float with 1 failed) SouravGoswami (Sourav Goswami)
  2021-09-22 21:04 ` [ruby-core:105380] " mame (Yusuke Endoh)
  2021-09-22 21:30 ` [ruby-core:105382] " SouravGoswami (Sourav Goswami)
@ 2021-09-23 17:55 ` jeremyevans0 (Jeremy Evans)
  2021-10-19  4:48 ` [ruby-core:105668] " mrkn (Kenta Murata)
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: jeremyevans0 (Jeremy Evans) @ 2021-09-23 17:55 UTC (permalink / raw)
  To: ruby-core

Issue #18187 has been updated by jeremyevans0 (Jeremy Evans).


I submitted a pull request to make Float::NAN.clamp return the receiver: https://github.com/ruby/ruby/pull/4884

However, like @mame, I'm not sure if it is more desirable to raise in this case.

----------------------------------------
Bug #18187: Float#clamp() returns ArgumentError (comparison of Float with 1 failed)
https://bugs.ruby-lang.org/issues/18187#change-93810

* Author: SouravGoswami (Sourav Goswami)
* Status: Open
* Priority: Normal
* ruby -v: ruby 3.0.2p107 (2021-07-07 revision 0db68f0233) [x86_64-linux]
* Backport: 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN
----------------------------------------
When I have a Float::NAN as a number, I expect all the method to work properly.

For example, `Float::NAN - 1` gives NAN. But Float::NAN.to_i raises FloatDomainError.

But in case of clamp(), Float::NAN.clamp(0, 100) returns `ArgumentError (comparison of Float with 1 failed)`

This error doesn't explain what's actually wrong. I didn't write the comparison to compare Float with 1. I didn't pass any invalid argument either. This error is a reflection of what's going on in the C level, which shouldn't appear to the user.

If I write a vanilla clamp() in ruby:

```
Float.define_method(:clamp2) { |min, max| self < min ? min : self > max ? max : self }
```

In this case, I can call it like this:

```
> 8.0.clamp2(10, 100)
=> 10
> 80.0.clamp2(10, 100)
=> 80.0
> 800.0.clamp2(10, 100)
=> 100
> Float::NAN.clamp2(10, 100)
=> NaN
```

As you can see, it just returns NAN. But in case of the built-in clamp, it raises the ArgumentError, even though my arguments are just correct. So this should handle this clamp() correctly, either returning the min value or `Float::NAN`.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [ruby-core:105668] [Ruby master Bug#18187] Float#clamp() returns ArgumentError (comparison of Float with 1 failed)
  2021-09-22 17:23 [ruby-core:105377] [Ruby master Bug#18187] Float#clamp() returns ArgumentError (comparison of Float with 1 failed) SouravGoswami (Sourav Goswami)
                   ` (2 preceding siblings ...)
  2021-09-23 17:55 ` [ruby-core:105398] " jeremyevans0 (Jeremy Evans)
@ 2021-10-19  4:48 ` mrkn (Kenta Murata)
  2021-10-19 11:12 ` [ruby-core:105679] " nobu (Nobuyoshi Nakada)
  2021-10-21  8:11 ` [ruby-core:105718] " matz (Yukihiro Matsumoto)
  5 siblings, 0 replies; 7+ messages in thread
From: mrkn (Kenta Murata) @ 2021-10-19  4:48 UTC (permalink / raw)
  To: ruby-core

Issue #18187 has been updated by mrkn (Kenta Murata).


I think it's OK to return NaN for all the cases of `Float::NAN.clamp`.

----------------------------------------
Bug #18187: Float#clamp() returns ArgumentError (comparison of Float with 1 failed)
https://bugs.ruby-lang.org/issues/18187#change-94167

* Author: SouravGoswami (Sourav Goswami)
* Status: Open
* Priority: Normal
* ruby -v: ruby 3.0.2p107 (2021-07-07 revision 0db68f0233) [x86_64-linux]
* Backport: 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN
----------------------------------------
When I have a Float::NAN as a number, I expect all the method to work properly.

For example, `Float::NAN - 1` gives NAN. But Float::NAN.to_i raises FloatDomainError.

But in case of clamp(), Float::NAN.clamp(0, 100) returns `ArgumentError (comparison of Float with 1 failed)`

This error doesn't explain what's actually wrong. I didn't write the comparison to compare Float with 1. I didn't pass any invalid argument either. This error is a reflection of what's going on in the C level, which shouldn't appear to the user.

If I write a vanilla clamp() in ruby:

```
Float.define_method(:clamp2) { |min, max| self < min ? min : self > max ? max : self }
```

In this case, I can call it like this:

```
> 8.0.clamp2(10, 100)
=> 10
> 80.0.clamp2(10, 100)
=> 80.0
> 800.0.clamp2(10, 100)
=> 100
> Float::NAN.clamp2(10, 100)
=> NaN
```

As you can see, it just returns NAN. But in case of the built-in clamp, it raises the ArgumentError, even though my arguments are just correct. So this should handle this clamp() correctly, either returning the min value or `Float::NAN`.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [ruby-core:105679] [Ruby master Bug#18187] Float#clamp() returns ArgumentError (comparison of Float with 1 failed)
  2021-09-22 17:23 [ruby-core:105377] [Ruby master Bug#18187] Float#clamp() returns ArgumentError (comparison of Float with 1 failed) SouravGoswami (Sourav Goswami)
                   ` (3 preceding siblings ...)
  2021-10-19  4:48 ` [ruby-core:105668] " mrkn (Kenta Murata)
@ 2021-10-19 11:12 ` nobu (Nobuyoshi Nakada)
  2021-10-21  8:11 ` [ruby-core:105718] " matz (Yukihiro Matsumoto)
  5 siblings, 0 replies; 7+ messages in thread
From: nobu (Nobuyoshi Nakada) @ 2021-10-19 11:12 UTC (permalink / raw)
  To: ruby-core

Issue #18187 has been updated by nobu (Nobuyoshi Nakada).


What about `Float#clamp`?

```diff
diff --git i/numeric.c w/numeric.c
index db2b2eb2793..12edb0f6006 100644
--- i/numeric.c
+++ w/numeric.c
@@ -2844,6 +2844,13 @@ num_step(int argc, VALUE *argv, VALUE from)
     return from;
 }
 
+static VALUE
+flo_clamp(int argc, VALUE *argv, VALUE x)
+{
+    if (isnan(RFLOAT_VALUE(x))) return x;
+    return rb_call_super(argc, argv);
+}
+
 static char *
 out_of_range_float(char (*pbuf)[24], VALUE val)
 {
@@ -5789,6 +5796,7 @@ Init_Numeric(void)
     rb_define_method(rb_cFloat, "finite?",   rb_flo_is_finite_p, 0);
     rb_define_method(rb_cFloat, "next_float", flo_next_float, 0);
     rb_define_method(rb_cFloat, "prev_float", flo_prev_float, 0);
+    rb_define_method(rb_cFloat, "clamp", flo_clamp, -1);
 }
 
 #undef rb_float_value
```

----------------------------------------
Bug #18187: Float#clamp() returns ArgumentError (comparison of Float with 1 failed)
https://bugs.ruby-lang.org/issues/18187#change-94179

* Author: SouravGoswami (Sourav Goswami)
* Status: Open
* Priority: Normal
* ruby -v: ruby 3.0.2p107 (2021-07-07 revision 0db68f0233) [x86_64-linux]
* Backport: 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN
----------------------------------------
When I have a Float::NAN as a number, I expect all the method to work properly.

For example, `Float::NAN - 1` gives NAN. But Float::NAN.to_i raises FloatDomainError.

But in case of clamp(), Float::NAN.clamp(0, 100) returns `ArgumentError (comparison of Float with 1 failed)`

This error doesn't explain what's actually wrong. I didn't write the comparison to compare Float with 1. I didn't pass any invalid argument either. This error is a reflection of what's going on in the C level, which shouldn't appear to the user.

If I write a vanilla clamp() in ruby:

```
Float.define_method(:clamp2) { |min, max| self < min ? min : self > max ? max : self }
```

In this case, I can call it like this:

```
> 8.0.clamp2(10, 100)
=> 10
> 80.0.clamp2(10, 100)
=> 80.0
> 800.0.clamp2(10, 100)
=> 100
> Float::NAN.clamp2(10, 100)
=> NaN
```

As you can see, it just returns NAN. But in case of the built-in clamp, it raises the ArgumentError, even though my arguments are just correct. So this should handle this clamp() correctly, either returning the min value or `Float::NAN`.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [ruby-core:105718] [Ruby master Bug#18187] Float#clamp() returns ArgumentError (comparison of Float with 1 failed)
  2021-09-22 17:23 [ruby-core:105377] [Ruby master Bug#18187] Float#clamp() returns ArgumentError (comparison of Float with 1 failed) SouravGoswami (Sourav Goswami)
                   ` (4 preceding siblings ...)
  2021-10-19 11:12 ` [ruby-core:105679] " nobu (Nobuyoshi Nakada)
@ 2021-10-21  8:11 ` matz (Yukihiro Matsumoto)
  5 siblings, 0 replies; 7+ messages in thread
From: matz (Yukihiro Matsumoto) @ 2021-10-21  8:11 UTC (permalink / raw)
  To: ruby-core

Issue #18187 has been updated by matz (Yukihiro Matsumoto).


I vote for keeping NaN raises exceptions.

Matz.


----------------------------------------
Bug #18187: Float#clamp() returns ArgumentError (comparison of Float with 1 failed)
https://bugs.ruby-lang.org/issues/18187#change-94218

* Author: SouravGoswami (Sourav Goswami)
* Status: Open
* Priority: Normal
* ruby -v: ruby 3.0.2p107 (2021-07-07 revision 0db68f0233) [x86_64-linux]
* Backport: 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN
----------------------------------------
When I have a Float::NAN as a number, I expect all the method to work properly.

For example, `Float::NAN - 1` gives NAN. But Float::NAN.to_i raises FloatDomainError.

But in case of clamp(), Float::NAN.clamp(0, 100) returns `ArgumentError (comparison of Float with 1 failed)`

This error doesn't explain what's actually wrong. I didn't write the comparison to compare Float with 1. I didn't pass any invalid argument either. This error is a reflection of what's going on in the C level, which shouldn't appear to the user.

If I write a vanilla clamp() in ruby:

```
Float.define_method(:clamp2) { |min, max| self < min ? min : self > max ? max : self }
```

In this case, I can call it like this:

```
> 8.0.clamp2(10, 100)
=> 10
> 80.0.clamp2(10, 100)
=> 80.0
> 800.0.clamp2(10, 100)
=> 100
> Float::NAN.clamp2(10, 100)
=> NaN
```

As you can see, it just returns NAN. But in case of the built-in clamp, it raises the ArgumentError, even though my arguments are just correct. So this should handle this clamp() correctly, either returning the min value or `Float::NAN`.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-10-21  8:11 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-22 17:23 [ruby-core:105377] [Ruby master Bug#18187] Float#clamp() returns ArgumentError (comparison of Float with 1 failed) SouravGoswami (Sourav Goswami)
2021-09-22 21:04 ` [ruby-core:105380] " mame (Yusuke Endoh)
2021-09-22 21:30 ` [ruby-core:105382] " SouravGoswami (Sourav Goswami)
2021-09-23 17:55 ` [ruby-core:105398] " jeremyevans0 (Jeremy Evans)
2021-10-19  4:48 ` [ruby-core:105668] " mrkn (Kenta Murata)
2021-10-19 11:12 ` [ruby-core:105679] " nobu (Nobuyoshi Nakada)
2021-10-21  8:11 ` [ruby-core:105718] " matz (Yukihiro Matsumoto)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).