-
-
Notifications
You must be signed in to change notification settings - Fork 762
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
[RFC]: refactor and add protocol support to stats/base/meanwd
#5652
Comments
👋 Important: PLEASE READ 👋This issue has been labeled as a good first issue and is available for anyone to work on. If this is your first time contributing to an open source project, some aspects of the development process may seem unusual, arcane, or some combination of both.
Before working on this issue and opening a pull request, please read the project's contributing guidelines. These guidelines and the associated development guide provide important information, including links to stdlib's Code of Conduct, license policy, and steps for setting up your local development environment. To reiterate, we strongly encourage you to refer to our contributing guides before beginning work on this issue. Failure to follow our guidelines significantly decreases the likelihood that you'll successfully contribute to stdlib and may result in automatic closure of a pull request without review. Setting up your local development environment is a critical first step, as doing so ensures that automated development processes for linting, license verification, and unit testing can run prior to authoring commits and pushing changes. If you would prefer to avoid manual setup, we provide pre-configured development containers for use locally or in GitHub Codespaces. We place a high value on consistency throughout the stdlib codebase. We encourage you to closely examine other packages in stdlib and attempt to emulate the practices and conventions found therein.
In short, the more effort you put in to ensure that your contribution looks and feels like stdlib—including variables names, bracket spacing, line breaks, etc—the more likely that your contribution will be reviewed and ultimately accepted. We encourage you to closely study the codebase before beginning work on this issue. ✨ Thank you again for your interest in stdlib, and we look forward to reviewing your future contributions. ✨ |
👋 Hi there! 👋 And thank you for opening your first issue! We will get back to you shortly. 🏃 💨 |
PR-URL: stdlib-js#6266 Closes: stdlib-js#5652 Co-authored-by: Athan Reines <kgryte@gmail.com> Reviewed-by: Athan Reines <kgryte@gmail.com> Signed-off-by: Athan Reines <kgryte@gmail.com>
PR-URL: stdlib-js#6266 Closes: stdlib-js#5652 Co-authored-by: Athan Reines <kgryte@gmail.com> Reviewed-by: Athan Reines <kgryte@gmail.com> Signed-off-by: Athan Reines <kgryte@gmail.com>
Description
This RFC proposes refactoring and adding accessor protocol support to
@stdlib/stats/base/meanwd
.For background on the accessor protocol, see https://blog.stdlib.io/introducing-the-accessor-protocol-for-array-like-objects/.
Examples of what we are looking for:
stats/base/min
: fbb31fb (package)stats/base/cumax
: 11f1341 (package)stats/base/cumin
: 528efd8 (package)stats/base/mediansorted
: 065f865 (package)stats/base/mskmax
: 417e653 (package)While the changes proposed in this RFC will not match the implementations found in the above packages, the packages should provide a conceptual idea of what is desired. Do not simply copy-paste the code found in those packages without reasoning about expected behavior and API design.
Key Points
Refactoring
When refactoring and cleaning up existing implementations, pay special attention to the changes made in the commits referred to above. You'll need to go line-by-line in
@stdlib/stats/base/meanwd
and compare with the changes made in the above commits.In particular, we're interested in
stats/base
use similar language for describing input parameters and behavior.ndarray
API.Accessor protocol
Prior to adding accessor protocol support, check whether the implementation has any strided array dependencies. For example, for certain algorithms for computing the mean, implementations will depend on algorithms for computing the sum. Ensure that the upstream implementations have support for accessor arrays. If not, you'll need to open a separate PR adding support in the upstream dependencies before working on this package.
When adding support for the accessor protocol, you'll need to do the following:
Create an accessors file
Create a new
lib/accessors.js
file. This file will contain an implementation which can handle accessor arrays.Modify the ndarray file
Modify the
lib/ndarray.js
file to delegate to the accessors implementation when one (or more) of the input arrays is an accessor array.Update tests
Modify the tests to include explicit tests for accessor arrays.
Run tests and other commands
For each of the following commands, please run them from the root stdlib repository directory (not the package folder!).
To run unit tests,
To run examples,
make examples EXAMPLES_FILTER=".*/stats/base/meanwd/.*"
To run benchmarks,
make benchmark BENCHMARKS_FILTER=".*/stats/base/meanwd/.*"
Create pull request
Provided all tests, examples, and benchmarks successfully execute and pass and that you've updated the package's documentation, you are now ready to open a pull request!
Notes
Checklist
[RFC]:
.The text was updated successfully, but these errors were encountered: