-
-
Notifications
You must be signed in to change notification settings - Fork 521
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
feat: add slice
method to array/fixed-endian-factory
#3188
base: develop
Are you sure you want to change the base?
Changes from all commits
7c59bd3
2aa24d2
763cef2
274405f
2e1175a
8b4abd9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are missing a separate benchmark file which evaluates performance as a function of array length. E.g., https://github.com/stdlib-js/stdlib/blob/4b1d77d2bd001d5970ce93230765a579fb41349d/lib/node_modules/%40stdlib/array/bool/benchmark/benchmark.slice.length.js |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
/** | ||
* @license Apache-2.0 | ||
* | ||
* Copyright (c) 2024 The Stdlib Authors. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
'use strict'; | ||
|
||
// MODULES // | ||
|
||
var bench = require( '@stdlib/bench' ); | ||
var pkg = require( './../package.json' ).name; | ||
var factory = require( './../lib' ); | ||
|
||
|
||
// VARIABLES // | ||
|
||
var Float64ArrayFE = factory( 'float64' ); | ||
|
||
|
||
// MAIN // | ||
|
||
bench( pkg+':slice', function benchmark( b ) { | ||
var out; | ||
var arr; | ||
var i; | ||
|
||
arr = new Float64ArrayFE( 'little-endian', [ 1.0, 2.0, 2.0, 1.0 ] ); | ||
|
||
b.tic(); | ||
for ( i = 0; i < b.iterations; i++ ) { | ||
out = arr.slice(1, 2); | ||
if ( out instanceof Float64ArrayFE === false ) { | ||
b.fail( 'should return a TypedArray' ); | ||
} | ||
} | ||
b.toc(); | ||
if ( out instanceof Float64ArrayFE === false ) { | ||
b.fail( 'should return a TypedArray' ); | ||
} | ||
b.pass( 'benchmark finished' ); | ||
b.end(); | ||
}); |
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -723,6 +723,54 @@ function factory( dtype ) { // eslint-disable-line max-lines-per-function, stdli | |||
return out.join( ',' ); | ||||
}); | ||||
|
||||
/** | ||||
* return sliced TypedArray. | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Your code should look very similar to
|
||||
* | ||||
* @private | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indentation is off. |
||||
* @name slice | ||||
* @memberof TypedArray.prototype | ||||
* @type {Function} | ||||
* @throws {TypeError} `this` must be a typed array instance | ||||
* @returns {TypedArray} string representation | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The description is incorrect. |
||||
*/ | ||||
|
||||
setReadOnly( TypedArray.prototype, 'slice', function slice() { | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Insert this method so that methods are listed in alphabetical order. |
||||
var value1; | ||||
var value2; | ||||
var nargs; | ||||
var temp; | ||||
var out; | ||||
var buf; | ||||
var i; | ||||
value1 = arguments[0]; | ||||
value2 = arguments[1]; | ||||
nargs = arguments.length; | ||||
if (value2 === null || nargs === 1) { | ||||
value2 = this._length; | ||||
} | ||||
if ( nargs < 0 || nargs > 2 || !isNonNegativeInteger( value1 ) || !isNonNegativeInteger( value2 ) ) { | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should accept negative integers, not just nonnegative integers. |
||||
throw new TypeError( 'invalid arguments.' ); | ||||
} | ||||
if ( value1 >= this._length || value2 > this._length || value1 >= value2 ) { | ||||
throw new RangeError( 'invalid range of arguments.' ); | ||||
} | ||||
if ( !isTypedArray( this ) ) { | ||||
throw new TypeError(format('invalid invocation. `this` is not %s %s.', CHAR2ARTICLE[ dtype[0]], CTOR_NAME)); | ||||
} | ||||
temp = []; | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You don't need this temporary array. As in
|
||||
buf = this._buffer; | ||||
for (i = value1; i < value2; i++) { | ||||
temp.push( buf[GETTER]( i * BYTES_PER_ELEMENT, this._isLE ) ); | ||||
} | ||||
|
||||
if (this._isLE) { | ||||
out = new TypedArray( 'little-endian', temp); | ||||
} else { | ||||
out = new TypedArray( 'big-endian', temp); | ||||
} | ||||
return out; | ||||
}); | ||||
|
||||
return TypedArray; | ||||
|
||||
/** | ||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Your tests should essentially follow the same structure as https://github.com/stdlib-js/stdlib/blob/4b1d77d2bd001d5970ce93230765a579fb41349d/lib/node_modules/%40stdlib/array/bool/test/test.slice.js. For every set of tests in that package, I would expect to see similar tests in this package. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,159 @@ | ||
/** | ||
* @license Apache-2.0 | ||
* | ||
* Copyright (c) 2024 The Stdlib Authors. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
'use strict'; | ||
|
||
// MODULES // | ||
|
||
var tape = require( 'tape' ); | ||
var hasOwnProp = require( '@stdlib/assert/has-own-property' ); | ||
var isFunction = require( '@stdlib/assert/is-function' ); | ||
var factory = require( './../lib' ); | ||
|
||
|
||
// TESTS // | ||
|
||
tape('main export is a function', function test( t ) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please follow stdlib spacing conventions consistently throughout this file. |
||
t.ok( true, __filename); | ||
t.strictEqual( typeof factory, 'function', 'main export is a function'); | ||
t.end(); | ||
}); | ||
|
||
tape('the function returns a function', function test( t ) { | ||
var ctor = factory('float64'); | ||
t.strictEqual( isFunction(ctor), true, 'returns expected value'); | ||
t.end(); | ||
}); | ||
|
||
tape( 'attached to the prototype of the returned function is a `slice` method', function test( t ) { | ||
var ctor = factory('float64'); | ||
t.strictEqual( hasOwnProp( ctor.prototype, 'slice'), true, 'returns expected value'); | ||
t.strictEqual( isFunction( ctor.prototype.slice), true, 'returns expected value'); | ||
t.end(); | ||
}); | ||
|
||
tape( 'the method throws an error if provided single argument which is not a non-negative integer', function test(t) { | ||
var values; | ||
var ctor; | ||
var arr; | ||
var i; | ||
|
||
ctor = factory( 'float64' ); | ||
arr = new ctor( 'little-endian', 10); | ||
|
||
values = [ | ||
'5', | ||
-4, | ||
3.14, | ||
NaN, | ||
true, | ||
false, | ||
null, | ||
void 0, | ||
{}, | ||
[] | ||
]; | ||
for (i = 0; i < values.length; i++) { | ||
t.throws( badValue(values[i]), TypeError, ' throws an error when provided ' + values[i]); | ||
} | ||
function badValue( value ) { | ||
return function badValue() { | ||
return arr.slice( value ); | ||
}; | ||
} | ||
t.end(); | ||
}); | ||
|
||
tape( 'the method throws an range error if provided two invalid argument ranges ', function test(t) { | ||
var values1; | ||
var values2; | ||
var ctor; | ||
var arr; | ||
var i; | ||
|
||
ctor = factory( 'float64' ); | ||
arr = new ctor( 'little-endian', [0, 1, 2, 3, 4] ); | ||
|
||
values1 = [ | ||
1, | ||
2, | ||
3, | ||
4, | ||
5, | ||
6, | ||
7, | ||
8, | ||
9, | ||
10 | ||
]; | ||
values2 = [ | ||
10, | ||
9, | ||
8, | ||
7, | ||
6, | ||
5, | ||
4, | ||
3, | ||
2, | ||
1 | ||
]; | ||
for ( i = 0; i < 10; i++ ) { | ||
t.throws( badValue( values1[i], values2[i] ), RangeError, 'throws an error when provided arg1 = ' + values1[i] + ', arg2 = ' + values2[i] ); | ||
} | ||
function badValue( v1, v2 ) { | ||
return function badValue() { | ||
return arr.slice( v1, v2 ); | ||
}; | ||
} | ||
|
||
t.end(); | ||
}); | ||
|
||
tape('the method returns same type array with instance with the parent array ', function test( t ) { | ||
var value1; | ||
var value2; | ||
var narr; | ||
var ctor; | ||
var arr; | ||
var i; | ||
|
||
ctor = factory( 'float64' ); | ||
arr = new ctor( 'little-endian', [1.0, 2.0, 3.0, 4.0, 5.2] ); | ||
|
||
value1 = [ | ||
0, | ||
1, | ||
2, | ||
3, | ||
4 | ||
]; | ||
value2 = [ | ||
1, | ||
2, | ||
3, | ||
4, | ||
5 | ||
]; | ||
|
||
for ( i=0; i < 5; i++ ) { | ||
narr = arr.slice( value1[0], value2[0] ); | ||
t.strictEqual( narr instanceof ctor, true, 'returns expected value' ); | ||
} | ||
t.end(); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See https://github.com/stdlib-js/stdlib/tree/4b1d77d2bd001d5970ce93230765a579fb41349d/lib/node_modules/%40stdlib/array/bool#booleanarrayprototypeslice-start-end- for what is excepted in terms of documentation. Additionally, this documentation should be inserted such that methods are listed in alphabetical order.