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.

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]: add tests for @stdlib/ndarray/base/nullary #2229

Closed
3 tasks done
headlessNode opened this issue May 4, 2024 · 11 comments · Fixed by #2663
Closed
3 tasks done

[RFC]: add tests for @stdlib/ndarray/base/nullary #2229

headlessNode opened this issue May 4, 2024 · 11 comments · Fixed by #2663
Assignees
Labels
Accepted RFC feature request which has been accepted. RFC Request for comments. Feature requests and proposed changes.

Comments

@headlessNode
Copy link
Contributor

Description

Currently, the @stdlib/ndarray/base/nullary/test/test.js only tests whether the main export is a function.

This RFC proposes to add tests to verify:

  • The first argument provided is an ndarray-like object

  • The input ndarray-like object contains valid data, dtype, shape, stride, offset, and order properties

  • No modification occurs if the data property of the input ndarray-like object is empty

  • The second argument provided is a function

  • The second argument returns a value compatible with the dtype of the output ndarray

  • The nullary function returns void

Related Issues

No.

Questions

No.

Other

No.

Checklist

  • I have read and understood the Code of Conduct.
  • Searched for existing issues and pull requests.
  • The issue name begins with RFC:.
@stdlib-bot
Copy link
Contributor

👋 Hi there! 👋

And thank you for opening your first issue! We will get back to you shortly. 🏃 💨

@headlessNode
Copy link
Contributor Author

@kgryte @Planeshifter @Pranavchiku I'd like to work on this issue.

@kgryte
Copy link
Member

kgryte commented May 24, 2024

@headlessNode Thanks for opening this issue. Tests would be good to add for @stdlib/ndarray/base/nullary, so this contribution would be welcome.

I would suggest, however, consulting @stdlib/strided/base/nullary to see how tests are done there, as @stdlib/ndarray/base/nullary is the ndarray analog to that package.

Of note, several of the verifications you mention are not necessary, such as verifying that an ndarray-like object is provided, that a function is provided, that the ndarray-like object has specified properties, etc. @stdlib/ndarray/base/nullary is a "base" package, and we assume that a user knows what they are doing and has provided valid arguments, as can be observed from the implementation of @stdlib/ndarray/base/nullary.

Instead, tests should verify iteration behavior for 0-11D ndarrays, including contiguous and non-contiguous views, etc. Additionally, tests should verify iteration behavior for ndarrays backed by accessor arrays (e.g., ndarrays have a complex-valued data type, such as complex64 or complex128).

Doing all this will be required in order to achieve 100% test coverage for this package, given its implementation: https://github.com/stdlib-js/stdlib/tree/develop/lib/node_modules/%40stdlib/ndarray/base/nullary/lib

@kgryte kgryte added RFC Request for comments. Feature requests and proposed changes. Needs Discussion Needs further discussion. labels May 24, 2024
@headlessNode
Copy link
Contributor Author

@kgryte I have gone through the @stdlib/strided/base/nullary and understand the "pattern" used there for testing. From my understanding, I would need to import the nullary0d up to nd and perform testing for each, including the tests for complex data types.

However, I noticed that I'm not assigned to this issue. I'd like to be assigned the issue to work on it.

@kgryte
Copy link
Member

kgryte commented Jun 2, 2024

@headlessNode Assigned.

@kgryte kgryte added Accepted RFC feature request which has been accepted. and removed Needs Discussion Needs further discussion. labels Jun 2, 2024
kgryte added a commit that referenced this issue Jul 14, 2024
PR-URL: #2350
Ref: #2229
Co-authored-by: Athan Reines <kgryte@gmail.com>
Reviewed-by: Athan Reines <kgryte@gmail.com>
@kgryte
Copy link
Member

kgryte commented Jul 14, 2024

Cross-linking #2350 (review) and copying over comment:

The primary missing tests needed at this point to achieve full test coverage are for blocked iteration. I've added tests for 2D blocked iteration.

Full test coverage can be achieved by extending the strategy for 2D blocked iteration to higher dimensions, utilizing (and shuffling around) singleton dimensions to exercise isolated conditionals at each nested loop level.

@headlessNode
Copy link
Contributor Author

@kgryte can I go ahead and work on adding the tests for higher dimensions?

@kgryte
Copy link
Member

kgryte commented Jul 14, 2024

@headlessNode Sure! See the "large arrays" tests for 2D for the idea.

@headlessNode
Copy link
Contributor Author

@kgryte Great. One more thing, why work on these in a new PR? I mean I could've added these in the PR we were working on.

@kgryte
Copy link
Member

kgryte commented Jul 14, 2024

@headlessNode No reason other than wanting to get something in. Some tests are better than no tests. And easier to review if the changes are isolated.

@headlessNode
Copy link
Contributor Author

@kgryte Makes sense. Thanks for the clarification.

kgryte added a commit that referenced this issue Jul 15, 2024
PR-URL: #2599
Ref: #2229
Co-authored-by: Athan Reines <kgryte@gmail.com>
Reviewed-by: Athan Reines <kgryte@gmail.com> 
Signed-off-by: Athan Reines <kgryte@gmail.com>
kgryte added a commit that referenced this issue Jul 19, 2024
PR-URL: #2609
Ref: #2229
Co-authored-by: Athan Reines <kgryte@gmail.com>
Reviewed-by: Athan Reines <kgryte@gmail.com>
kgryte added a commit that referenced this issue Jul 21, 2024
PR-URL: #2634
Ref: #2229
Co-authored-by: Athan Reines <kgryte@gmail.com>
Reviewed-by: Athan Reines <kgryte@gmail.com>
kgryte added a commit that referenced this issue Jul 21, 2024
PR-URL: #2644
Ref: #2229
Co-authored-by: Athan Reines <kgryte@gmail.com>
Reviewed-by: Athan Reines <kgryte@gmail.com>
kgryte added a commit that referenced this issue Jul 22, 2024
PR-URL: #2645
Ref: #2229
Co-authored-by: Athan Reines <kgryte@gmail.com>
Reviewed-by: Athan Reines <kgryte@gmail.com>
kgryte added a commit that referenced this issue Jul 24, 2024
PR-URL: #2652
Ref: #2229
Co-authored-by: Athan Reines <kgryte@gmail.com>
Reviewed-by: Athan Reines <kgryte@gmail.com>
kgryte added a commit that referenced this issue Jul 25, 2024
PR-URL: #2655
Ref: #2229
Co-authored-by: Athan Reines <kgryte@gmail.com>
Reviewed-by: Athan Reines <kgryte@gmail.com>
@kgryte kgryte closed this as completed in cace2b4 Jul 25, 2024
gunjjoshi pushed a commit to gunjjoshi/stdlib that referenced this issue Aug 21, 2024
PR-URL: stdlib-js#2634
Ref: stdlib-js#2229
Co-authored-by: Athan Reines <kgryte@gmail.com>
Reviewed-by: Athan Reines <kgryte@gmail.com>
gunjjoshi pushed a commit to gunjjoshi/stdlib that referenced this issue Aug 21, 2024
PR-URL: stdlib-js#2644
Ref: stdlib-js#2229
Co-authored-by: Athan Reines <kgryte@gmail.com>
Reviewed-by: Athan Reines <kgryte@gmail.com>
gunjjoshi pushed a commit to gunjjoshi/stdlib that referenced this issue Aug 21, 2024
PR-URL: stdlib-js#2645
Ref: stdlib-js#2229
Co-authored-by: Athan Reines <kgryte@gmail.com>
Reviewed-by: Athan Reines <kgryte@gmail.com>
gunjjoshi pushed a commit to gunjjoshi/stdlib that referenced this issue Aug 21, 2024
PR-URL: stdlib-js#2652
Ref: stdlib-js#2229
Co-authored-by: Athan Reines <kgryte@gmail.com>
Reviewed-by: Athan Reines <kgryte@gmail.com>
gunjjoshi pushed a commit to gunjjoshi/stdlib that referenced this issue Aug 21, 2024
PR-URL: stdlib-js#2655
Ref: stdlib-js#2229
Co-authored-by: Athan Reines <kgryte@gmail.com>
Reviewed-by: Athan Reines <kgryte@gmail.com>
gunjjoshi pushed a commit to gunjjoshi/stdlib that referenced this issue Aug 21, 2024
PR-URL: stdlib-js#2663
Closes: stdlib-js#2229
Co-authored-by: Athan Reines <kgryte@gmail.com>
Reviewed-by: Athan Reines <kgryte@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepted RFC feature request which has been accepted. RFC Request for comments. Feature requests and proposed changes.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants