Skip to content

Commit f738932

Browse files
Fix range contains (#14011)
# Description This PR changes the range contains logic to take the step into account. ```nushell # before 2 in 1..3.. # true # now 2 in 1..3.. # false ``` --- I encountered another issue while adding tests. Due to floating point precision, `2.1 in 1..1.1..3` will return `false`. The floating point error is even bigger than `f64::EPSILON` (`0.09999999999999876` vs `2.220446049250313e-16`). This issue disappears with bigger numbers. I tried a different algorithm (checking if the estimated number of steps is close enough to any integer) but the results are still pretty bad: ```rust let n_steps = (value - self.start) / self.step; // 14.999999999999988 (n_steps - n_steps.round()).abs() < f64::EPSILON // returns false ``` Maybe it can be shipped like this, the REPL already has floating point errors (`1.1 - 1` returns `0.10000000000000009`). Or maybe there's a way to fix this that I didn't think of. I'm open to ideas! But in any case performing this kind of checks on a range of floats seems more niche than doing it on a range of ints. # User-Facing Changes Code that depended on this behavior to check if a number is between `start` and `end` will potentially return a different value. # Tests + Formatting # After Submitting
1 parent 4968b6b commit f738932

File tree

2 files changed

+79
-25
lines changed

2 files changed

+79
-25
lines changed

crates/nu-protocol/src/value/range.rs

Lines changed: 38 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -95,20 +95,27 @@ mod int_range {
9595

9696
pub fn contains(&self, value: i64) -> bool {
9797
if self.step < 0 {
98-
value <= self.start
99-
&& match self.end {
100-
Bound::Included(end) => value >= end,
101-
Bound::Excluded(end) => value > end,
102-
Bound::Unbounded => true,
103-
}
98+
// Decreasing range
99+
if value > self.start {
100+
return false;
101+
}
102+
match self.end {
103+
Bound::Included(end) if value < end => return false,
104+
Bound::Excluded(end) if value <= end => return false,
105+
_ => {}
106+
};
104107
} else {
105-
self.start <= value
106-
&& match self.end {
107-
Bound::Included(end) => value <= end,
108-
Bound::Excluded(end) => value < end,
109-
Bound::Unbounded => true,
110-
}
108+
// Increasing range
109+
if value < self.start {
110+
return false;
111+
}
112+
match self.end {
113+
Bound::Included(end) if value > end => return false,
114+
Bound::Excluded(end) if value >= end => return false,
115+
_ => {}
116+
};
111117
}
118+
(value - self.start) % self.step == 0
112119
}
113120

114121
pub fn into_range_iter(self, signals: Signals) -> Iter {
@@ -330,20 +337,27 @@ mod float_range {
330337

331338
pub fn contains(&self, value: f64) -> bool {
332339
if self.step < 0.0 {
333-
value <= self.start
334-
&& match self.end {
335-
Bound::Included(end) => value >= end,
336-
Bound::Excluded(end) => value > end,
337-
Bound::Unbounded => true,
338-
}
340+
// Decreasing range
341+
if value > self.start {
342+
return false;
343+
}
344+
match self.end {
345+
Bound::Included(end) if value <= end => return false,
346+
Bound::Excluded(end) if value < end => return false,
347+
_ => {}
348+
};
339349
} else {
340-
self.start <= value
341-
&& match self.end {
342-
Bound::Included(end) => value <= end,
343-
Bound::Excluded(end) => value < end,
344-
Bound::Unbounded => true,
345-
}
350+
// Increasing range
351+
if value < self.start {
352+
return false;
353+
}
354+
match self.end {
355+
Bound::Included(end) if value >= end => return false,
356+
Bound::Excluded(end) if value > end => return false,
357+
_ => {}
358+
};
346359
}
360+
((value - self.start) % self.step).abs() < f64::EPSILON
347361
}
348362

349363
pub fn into_range_iter(self, signals: Signals) -> Iter {

tests/repl/test_ranges.rs

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,24 @@ fn int_in_inc_range() -> TestResult {
77

88
#[test]
99
fn int_in_dec_range() -> TestResult {
10-
run_test(r#"1 in 9.42..-4"#, "true")
10+
run_test(r#"1 in 9..-4.42"#, "true")
1111
}
1212

1313
#[test]
1414
fn int_in_exclusive_range() -> TestResult {
1515
run_test(r#"3 in 0..<3"#, "false")
1616
}
1717

18+
#[test]
19+
fn float_in_inc_range() -> TestResult {
20+
run_test(r#"1.58 in -4.42..9"#, "true")
21+
}
22+
23+
#[test]
24+
fn float_in_dec_range() -> TestResult {
25+
run_test(r#"1.42 in 9.42..-4.42"#, "true")
26+
}
27+
1828
#[test]
1929
fn non_number_in_range() -> TestResult {
2030
fail_test(r#"'a' in 1..3"#, "subset comparison is not supported")
@@ -34,3 +44,33 @@ fn range_and_reduction() -> TestResult {
3444
fn zip_ranges() -> TestResult {
3545
run_test(r#"1..3 | zip 4..6 | get 2.1"#, "6")
3646
}
47+
48+
#[test]
49+
fn int_in_stepped_range() -> TestResult {
50+
run_test(r#"7 in 1..3..15"#, "true")
51+
}
52+
53+
#[test]
54+
fn int_in_unbounded_stepped_range() -> TestResult {
55+
run_test(r#"1000001 in 1..3.."#, "true")
56+
}
57+
58+
#[test]
59+
fn int_not_in_unbounded_stepped_range() -> TestResult {
60+
run_test(r#"2 in 1..3.."#, "false")
61+
}
62+
63+
#[test]
64+
fn float_in_stepped_range() -> TestResult {
65+
run_test(r#"5.5 in 1..1.5..10"#, "true")
66+
}
67+
68+
#[test]
69+
fn float_in_unbounded_stepped_range() -> TestResult {
70+
run_test(r#"100.5 in 1..1.5.."#, "true")
71+
}
72+
73+
#[test]
74+
fn float_not_in_unbounded_stepped_range() -> TestResult {
75+
run_test(r#"2.1 in 1.2..3.."#, "false")
76+
}

0 commit comments

Comments
 (0)