-
-
Notifications
You must be signed in to change notification settings - Fork 508
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 iter/cuevery
#2664
feat: add iter/cuevery
#2664
Conversation
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.
👋 Hi there! 👋
And thank you for opening your first pull request! We will review it shortly. 🏃 💨
@kgryte can you review this? |
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.
This file should not be modified.
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.
This file is empty.
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.
This file does not conform to our REPL text conventions. Please read the REPL text guide.
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.
Missing license header.
* v = it.next().done; | ||
* // returns true | ||
*/ | ||
declare function iterCuEvery( iterator: Iterator ): Iterator; |
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.
You can improve return value specificity by using the TypedIterator
interface.
*/ | ||
|
||
import iterCuEvery from './index'; | ||
//import isIteratorLike from '@stdlib/assert/is-iterator-like'; |
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.
Why is this here?
|
||
// The function returns an iterator... | ||
{ | ||
iterCuEvery(iterator()); // $ExpectType Iterator |
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.
Incorrect spacing and indentation throughout this file.
|
||
// The compiler throws an error if the function is provided a value other than an iterator protocol-compliant object... | ||
{ | ||
iterCuEvery(5 as any); // $ExpectError |
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.
Why are you casting?
iterCuEvery(undefined as any); // $ExpectError | ||
iterCuEvery([] as any); // $ExpectError | ||
iterCuEvery({} as any); // $ExpectError | ||
//iterCuEvery((x: number): number => x as any as Iterator); // $ExpectError |
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.
Why is this commented out?
// Test iterator with Symbol.iterator if supported | ||
{ | ||
const it = iterator(); | ||
if (Symbol.iterator in it) { | ||
iterCuEvery(it); // $ExpectType Iterator | ||
} | ||
} |
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.
This should be removed.
}); | ||
|
||
// Create an iterator which applies a threshold to generated numbers: | ||
var it = iterMap( rand, function threshold( r ) { |
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.
Do not use function expressions.
|
||
// Create an iterator which generates uniformly distributed pseudorandom numbers: | ||
var rand = randu({ | ||
'iter': 100 |
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.
This file has incorrect indentation.
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.
This file is empty.
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.
Actually, this file should be removed.
* @module @stdlib/iter-cuevery | ||
* | ||
* @example | ||
* var array2iterator = require( '@stdlib/array-to-iterator' ); | ||
* var iterCuEvery = require( '@stdlib/iter-cuevery' ); |
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.
Incorrect module names. See other packages within the monorepo.
|
||
|
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.
Why are these spaces here?
if (!isIteratorLike(iterator)) { | ||
throw new TypeError(format('invalid argument. Must provide an iterator. Value: `%s`.', iterator)); | ||
} |
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.
Incorrect spacing and indentation. It is clear that you failed to setup your local development environment, as our linting would have caught these errors. Please read our contributing guidelines.
} | ||
if (!v.value) { | ||
FLG = false; | ||
done = true; // Short-circuit once a falsy value is found |
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.
This is not correct behavior. This is a cumulative iterator and should have the same length as the source iterator.
|
||
return iter; | ||
|
||
function next() { |
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.
You're missing JSDoc comments.
var FLG = true; // Assume all values are truthy initially | ||
var done = false; | ||
|
||
var iter = { |
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.
This isn't how we do variable declarations in source files.
}; | ||
} | ||
|
||
function finish(value) { |
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.
JSDoc comments.
}; | ||
} | ||
|
||
function factory() { |
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.
JSDoc comments.
|
||
// EXPORTS // | ||
|
||
module.exports = iterCuEvery; |
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.
You did not setup EditorConfig per the contributing guidelines.
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.
Why is this file here? Remove.
"url": "https://github.com/stdlib-js/stdlib/graphs/contributors" | ||
} | ||
], | ||
"license": "ISC", |
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.
What is this? This package.json
is not how we do things. See other packages.
// /** | ||
// * @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. | ||
// */ | ||
|
||
|
||
|
||
|
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.
Why are you commenting out our license header?
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.
This file does not follow project conventions.
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.
Why did you delete this file?
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.
This PR needs significant modifications before it can be reviewed again and considered for inclusion in the project.
This pull request has been automatically closed because it has been inactive for an extended period after changes were requested. If you still wish to pursue this contribution, feel free to reopen the pull request or submit a new one. We appreciate your interest in contributing to stdlib! |
Resolves #2332.
Description
This pull request:
iterCuevery
Related Issues
This pull request:
@stdlib/iter/cuevery
#2332Questions
No.
Other
Checklist
@stdlib-js/reviewers