Skip to content

Commit fafc0e2

Browse files
author
Mathieu Bour
committed
feat(range): delegate RangeToken calculations to the bignumber.js
fix: rounding issues (#1, #3) tests: refine tests (remove `it.each` when possible), add #3 test case Closes: #3
1 parent 15101f6 commit fafc0e2

9 files changed

Lines changed: 107 additions & 37 deletions

File tree

.standard-commitrc.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"types": ["feat", "fix", "chore", "build", "ci", "docs", "perf", "refactor", "revert", "style", "test"],
3+
"promptScope": "suggest",
4+
"scopes": "staged"
5+
}

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
# Changelog
22

3+
## [Version 0.2.0](https://github.com/csquare-ai/crossp/releases/tag/0.2.0)
4+
5+
- **feat**: delegate `RangeToken` calculations to the `bignumber.js` library
6+
- **fix**: rounding issues ([#1](https://github.com/csquare-ai/crossp/issues/1), [#3](https://github.com/csquare-ai/crossp/issues/3))
7+
- **tests**: refine tests (remove `it.each` when possible), add [#3](https://github.com/csquare-ai/crossp/issues/3) test case
8+
39
## [Version 0.1.2 - Fix types directory](https://github.com/csquare-ai/crossp/releases/tag/0.1.2)
410

511
- **fix**: set types directory to `dist/types`

package-lock.json

Lines changed: 18 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@csquare/crossp",
3-
"version": "0.1.2",
3+
"version": "0.2.0",
44
"description": "Generate commands from a single one using csquare syntax.",
55
"keywords": [
66
"csquare",
@@ -53,5 +53,8 @@
5353
"engines": {
5454
"node": ">=10"
5555
},
56-
"prettier": "@csquare/prettier-config"
56+
"prettier": "@csquare/prettier-config",
57+
"dependencies": {
58+
"bignumber.js": "^9.0.1"
59+
}
5760
}

src/crossp.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,11 @@ export function crossp(command: string): string[] {
3232
}
3333

3434
const replacements = cartesian(...tokens.map((t) => t.values));
35-
const truncRegex = /(?!\d*\..)0{2}\d*/g;
3635
const result: string[] = replacements.map((line) => {
3736
let acc = work;
3837

3938
line.forEach((value, index) => {
40-
acc = acc.replace(placeholder(index), value.replace(truncRegex, ''));
39+
acc = acc.replace(placeholder(index), value);
4140
});
4241

4342
return acc;

src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,5 @@ import crossp from './crossp';
22

33
// Public package API
44
export { crossp } from './crossp';
5+
export { limits } from './limits';
56
export default crossp;

src/lib/cartesian.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,18 @@
1+
import { limits } from '../limits';
2+
13
/**
24
* @see https://stackoverflow.com/a/43053803/3728261
35
*/
4-
import { limits } from '../limits';
5-
66
export default function cartesian(...values: string[][]): string[][] {
77
// @ts-ignore
88
let out: string[][] = values.reduce((matrix: string[][], current: string[]) => {
9-
if (matrix.length > limits.outputs) {
9+
const result = matrix.flatMap((d: string | string[]) => current.map((e: string) => [d, e].flat()));
10+
11+
if (result.length > limits.outputs) {
1012
throw new RangeError(`Command exceeds the maximum of ${limits.outputs} outputs`);
1113
}
1214

13-
return matrix.flatMap((d: string | string[]) => current.map((e: string) => [d, e].flat()));
15+
return result;
1416
});
1517

1618
if (out?.[0] && typeof out[0] === 'string') {

src/tokens/range-token.ts

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
1+
import BigNumber from 'bignumber.js';
12
import { limits } from '../limits';
23
import { Token } from './token';
34

45
export class RangeToken implements Token {
56
private readonly FLOAT_REGEX = /[+-]?(?:\d*[.])?\d+/;
6-
readonly start: number;
7-
readonly end: number;
8-
readonly increment: number;
7+
readonly start: BigNumber;
8+
readonly end: BigNumber;
9+
readonly increment: BigNumber;
910
readonly values: string[] = [];
1011

1112
constructor(readonly raw: string) {
@@ -23,27 +24,27 @@ export class RangeToken implements Token {
2324
throw new SyntaxError('Non numeric range parameters');
2425
}
2526

26-
this.start = parseFloat(parts[0]);
27-
this.end = parseFloat(parts[1]);
28-
this.increment = Math.abs(parts?.[2] && parts[2].length > 0 ? parseFloat(parts[2]) : 1);
27+
this.start = new BigNumber(parts[0]);
28+
this.end = new BigNumber(parts[1]);
29+
this.increment = parts?.[2] && parts[2].length > 0 ? new BigNumber(parts[2]).abs() : new BigNumber(1);
2930

30-
if (this.increment === 0) {
31+
if (this.increment.eq(0)) {
3132
throw new RangeError('Increment cannot be equal to zero');
3233
}
3334

34-
if (this.start > this.end) {
35+
if (this.start.gt(this.end)) {
3536
[this.start, this.end] = [this.end, this.start];
3637
}
3738

38-
if ((this.end - this.start) / this.increment > limits.range) {
39+
if (this.end.minus(this.start).div(this.increment).gt(limits.range)) {
3940
throw new RangeError(`Range operator exceeds the maximum allowed domain of ${limits.range} values`);
4041
}
4142

42-
let current = this.start;
43+
let current = new BigNumber(this.start);
4344

44-
while (current <= this.end) {
45+
while (current.lte(this.end)) {
4546
this.values.push(current.toString(10));
46-
current += this.increment;
47+
current = current.plus(this.increment);
4748
}
4849
}
4950
}

tests/crossp.spec.ts

Lines changed: 52 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -93,28 +93,47 @@ describe('compiler', () => {
9393
],
9494
])('should compile %p using multi-statements', expectToEqual);
9595

96-
it.each([
97-
['python train.py --epochs=%[]%', 'Empty comma-separated list'],
98-
['python train.py --epochs=%(10,%[1,2,3]%,2)%', 'Invalid number of arguments for range'],
99-
['python train.py --epochs=%(in,out,1)%', 'Non numeric range parameters'],
100-
['python train.py --epochs=%(1,10,0)%', 'Increment cannot be equal to zero'],
101-
])('should throw a syntax error for %p', expectToThrow);
96+
it('should throw if there is an empty comma-separated list', () => {
97+
expectToThrow('python train.py --epochs=%[]%', 'Empty comma-separated list');
98+
});
10299

103-
it.each([
104-
[
100+
it('should throw if there is invalid arguments for the range operator', () => {
101+
expectToThrow('python train.py --epochs=%(10,%[1,2,3]%,2)%', 'Invalid number of arguments for range');
102+
});
103+
104+
it('should throw if the range operator get non-numeric operators', () => {
105+
expectToThrow('python train.py --epochs=%(in,out,1)%', 'Non numeric range parameters');
106+
});
107+
108+
it('should throw if the range operator is equal to zero', () => {
109+
expectToThrow('python train.py --epochs=%(1,10,0)%', 'Increment cannot be equal to zero');
110+
});
111+
112+
it(`should throw if a range operator exceeds the maximum allowed domain of ${limits.range} values`, () => {
113+
expectToThrow(
114+
'python train.py --foo=%(0,1,0.0000001)%',
115+
`Range operator exceeds the maximum allowed domain of ${limits.range} values`,
116+
);
117+
});
118+
119+
it(`should throw if the command generate more than the allowed limit of ${limits.outputs} outputs`, () => {
120+
expectToThrow(
121+
'python train.py --foo=%(0,100)% --bar=%(0,100)% --baz%(0,100)%',
122+
`Command exceeds the maximum of ${limits.outputs} outputs`,
123+
);
124+
});
125+
126+
it(`should throw if the command exceeds the maximum allowed length of ${limits.length} characters`, () => {
127+
expectToThrow(
105128
`python train.py ${new Array(limits.length * 2).join('#')}`,
106129
`Command exceeds the maximum allowed length of ${limits.length} characters`,
107-
],
108-
[
109-
'python train.py --foo=%(0,1,0.000001)%',
110-
`Range operator exceeds the maximum allowed domain of ${limits.range} values`,
111-
],
112-
])('should not compile %p and be rate limited', expectToThrow);
130+
);
131+
});
113132

114133
/**
115134
* @link https://github.com/csquare-ai/crossp/issues/1
116135
*/
117-
it('should not have round issues', () => {
136+
it('should not have rounding issues with %(0.1,0.5,0.1)%', () => {
118137
expectToEqual('python --train.py --lr=%(0.1,0.5,0.1)%', [
119138
'python --train.py --lr=0.1',
120139
'python --train.py --lr=0.2',
@@ -123,4 +142,22 @@ describe('compiler', () => {
123142
'python --train.py --lr=0.5',
124143
]);
125144
});
145+
146+
/**
147+
* @link https://github.com/csquare-ai/crossp/issues/3
148+
*/
149+
it('should not have rounding issues with %(10,100,10)%', () => {
150+
expectToEqual('python --train.py --lr=%(10,100,10)%', [
151+
'python --train.py --lr=10',
152+
'python --train.py --lr=20',
153+
'python --train.py --lr=30',
154+
'python --train.py --lr=40',
155+
'python --train.py --lr=50',
156+
'python --train.py --lr=60',
157+
'python --train.py --lr=70',
158+
'python --train.py --lr=80',
159+
'python --train.py --lr=90',
160+
'python --train.py --lr=100',
161+
]);
162+
});
126163
});

0 commit comments

Comments
 (0)