Skip to content
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.

Sign up for GitHub

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

Jump to bottom

docs: improve examples of ndarray/iter namespace #1686

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

docs: improve examples of ndarray/iter namespace #1686

wants to merge 6 commits into from

Conversation

performant23
Copy link
Contributor

@performant23 performant23 commented Mar 4, 2024 •
edited by kgryte
Loading

Updated examples to span over the functionalities implemented in the namespace.

Resolves #1589

Description

What is the purpose of this pull request?

Provide better examples for the namespace to demonstrate its capabilities and usage.

This pull request:

  • Imports different functionalities inside ndarray/iter to form useful usecases

Related Issues

Does this pull request have any related issues?

This pull request:

Questions

Any questions for reviewers of this pull request?

No.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

No.

Checklist

Please ensure the following tasks are completed before submitting this pull request.

@stdlib-js/reviewers

@performant23
docs: improve README examples of ndarray/iter namespace
b36dc4a 
Updated examples to span over the functionalities implemented in the namespace.

Addresses: stdlib-js#1589
@performant23
Copy link
Contributor Author

Hello, thought I should just push a draft PR for one namespace and receive feedback before moving to another in the package in ndarray!
Also, I have these questions:

  1. Do we include the outputs of the examples as comments for README.md?
  2. Do we need to format the output in some particular way? That is, in the second example, the log is like: Index: [ 0, 0 ] Value: 1, and for the last one, we print the initial type of the object like: Original ndarray: ndarray( 'generic', [ 1, 2, 3, 4, 5, 6, 7, 8, 9 ], [ 3, 3 ], [ 3, 1 ], 0, 'row-major' ). So, is that necessary or we should just always log the final output?

Thank you so much!!

@performant23
Update README.md
aa1b0e4 
Update the copyright year

Signed-off-by: Rutam <[email protected]>
@kgryte kgryte added Enhancement Issue or pull request for enhancing existing functionality. Needs Review A pull request which needs code review. Documentation Improvements, additions, or changes to documentation. labels Mar 4, 2024
kgryte
kgryte reviewed Mar 4, 2024
View reviewed changes
lib/node_modules/@stdlib/ndarray/iter/README.md Outdated
Comment on lines 74 to 78
// Example: Get an array consisting of keys/properties inside ns.
var objectKeys = require( '@stdlib/utils/keys' );
var ns = require( '@stdlib/ndarray/iter' );

console.log( objectKeys( ns ) );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer modifying the existing example code. Project conventions are to not create multiple separate Markdown code blocks. Prefer instead a single Markdown code block with various example usage.

kgryte
kgryte reviewed Mar 4, 2024
View reviewed changes
lib/node_modules/@stdlib/ndarray/iter/README.md Outdated
var iterEntries = ns.nditerEntries( x );
var entry;

for ( entry of iterEntries ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ES5.

kgryte
kgryte reviewed Mar 4, 2024
View reviewed changes
lib/node_modules/@stdlib/ndarray/iter/README.md Outdated
```

```javascript
// Example: Iterate over each row of the ndarray
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These examples seem to just be pulling from the READMEs of the child packages. I think we would prefer something fresh, which ideally combines the functionality of multiple packages to demonstrate how they can all work together.

@kgryte kgryte added the Needs Changes Pull request which needs changes before being merged. label Mar 4, 2024
@Planeshifter Planeshifter changed the title docs: improve README examples of ndarray/iter namespace docs: improve README examples of ndarray/iter namespace Mar 4, 2024
@performant23
docs: apply suggestions from code review
440c045 
Updated example for the namespace using functionalities from stats, random, and ndarray

Fixes: stdlib-js#1589
@performant23
Merge remote-tracking branch 'origin/docs/improve_README_examples_of_…
9e73fd3 
…ndarray/iter_namespace' into docs/improve_README_examples_of_ndarray/iter_namespace
@performant23
Copy link
Contributor Author

performant23 commented Mar 4, 2024 •
edited
Loading

Thank you so much for the review, @kgryte! I have modified the commit to include an example which uses functionalities from other packages! Please do let me know if there are any further changes necessary.

Also, I found that packages followed the convention of including comments below code. Since I was getting the below message, I left a line between the code and the necessary comments. Please do let me know in case we need to include corresponding comments above the code.

Missing empty line before comment  stdlib/empty-line-before-comment

@Planeshifter Planeshifter changed the title docs: improve README examples of ndarray/iter namespace docs: improve examples of ndarray/iter namespace Sep 8, 2024
@Planeshifter Planeshifter self-requested a review September 8, 2024 02:28
@kgryte
Copy link
Member

kgryte commented Nov 25, 2024

/stdlib merge

@stdlib-bot stdlib-bot added the bot: In Progress Pull request is currently awaiting automation. label Nov 25, 2024
@kgryte kgryte removed Needs Changes Pull request which needs changes before being merged. bot: In Progress Pull request is currently awaiting automation. labels Nov 25, 2024
@stdlib-bot
Copy link
Contributor

Coverage Report

Package Statements Branches Functions Lines
ndarray/iter $\color{green}168/168$
$\color{green}+100.00\%$ 		$\color{green}1/1$
$\color{green}+100.00\%$ 		$\color{green}0/0$
$\color{green}+100.00\%$ 		$\color{green}168/168$
$\color{green}+100.00\%$

The above coverage report was generated for the changes in this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kgryte kgryte kgryte left review comments

@Planeshifter Planeshifter Awaiting requested review from Planeshifter

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
Documentation Improvements, additions, or changes to documentation. Enhancement Issue or pull request for enhancing existing functionality. Needs Review A pull request which needs code review.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[RFC]: improve README examples of ndarray/iter namespace
3 participants
@performant23 @kgryte @stdlib-bot