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

feat : add @stdlib/assert/is-nonnegative-finite #1354

Merged
merged 26 commits into from
Feb 23, 2024
Merged

feat : add @stdlib/assert/is-nonnegative-finite #1354

merged 26 commits into from
Feb 23, 2024

Conversation

marsian83
Copy link
Contributor

@marsian83 marsian83 commented Feb 23, 2024

Resolves #1344 .

Description

This PR adds the package @stdlib/assert/is-nonnegative-finite.

The package implements assertion for nonnegative number except positive infinity.

Alias: isNonNegativeFinite

Related Issues

Questions

I have used the @stdlib/asset/is-finite in order to achieve the desired result. Is this considered good practice, what are the guidelines for using internal packages, when and where is it ideal.

Checklist


@stdlib-js/reviewers

Copy link
Contributor

@stdlib-bot stdlib-bot left a 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. 🏃 💨

marsian83 and others added 15 commits February 23, 2024 20:07
Co-authored-by: Philipp Burckhardt <pburckhardt@outlook.com>
Signed-off-by: Spandan Barve <114365550+marsian83@users.noreply.github.com>
Co-authored-by: Philipp Burckhardt <pburckhardt@outlook.com>
Signed-off-by: Spandan Barve <114365550+marsian83@users.noreply.github.com>
Co-authored-by: Philipp Burckhardt <pburckhardt@outlook.com>
Signed-off-by: Spandan Barve <114365550+marsian83@users.noreply.github.com>
Co-authored-by: Philipp Burckhardt <pburckhardt@outlook.com>
Signed-off-by: Spandan Barve <114365550+marsian83@users.noreply.github.com>
Co-authored-by: Philipp Burckhardt <pburckhardt@outlook.com>
Signed-off-by: Spandan Barve <114365550+marsian83@users.noreply.github.com>
Co-authored-by: Philipp Burckhardt <pburckhardt@outlook.com>
Signed-off-by: Spandan Barve <114365550+marsian83@users.noreply.github.com>
Co-authored-by: Philipp Burckhardt <pburckhardt@outlook.com>
Signed-off-by: Spandan Barve <114365550+marsian83@users.noreply.github.com>
Co-authored-by: Philipp Burckhardt <pburckhardt@outlook.com>
Signed-off-by: Spandan Barve <114365550+marsian83@users.noreply.github.com>
Co-authored-by: Philipp Burckhardt <pburckhardt@outlook.com>
Signed-off-by: Spandan Barve <114365550+marsian83@users.noreply.github.com>
Co-authored-by: Philipp Burckhardt <pburckhardt@outlook.com>
Signed-off-by: Spandan Barve <114365550+marsian83@users.noreply.github.com>
Co-authored-by: Philipp Burckhardt <pburckhardt@outlook.com>
Signed-off-by: Spandan Barve <114365550+marsian83@users.noreply.github.com>
Co-authored-by: Philipp Burckhardt <pburckhardt@outlook.com>
Signed-off-by: Spandan Barve <114365550+marsian83@users.noreply.github.com>
Co-authored-by: Philipp Burckhardt <pburckhardt@outlook.com>
Signed-off-by: Spandan Barve <114365550+marsian83@users.noreply.github.com>
Co-authored-by: Philipp Burckhardt <pburckhardt@outlook.com>
Signed-off-by: Spandan Barve <114365550+marsian83@users.noreply.github.com>
Co-authored-by: Philipp Burckhardt <pburckhardt@outlook.com>
Signed-off-by: Spandan Barve <114365550+marsian83@users.noreply.github.com>
@Planeshifter
Copy link
Member

Planeshifter commented Feb 23, 2024

@marsian83

I have used the @stdlib/asset/is-finite in order to achieve the desired result. Is this considered good practice, what are the guidelines for using internal packages, when and where is it ideal.

Yes, we generally want to explicitly use internal packages instead of native methods. This makes the dependencies of the packages explicit and what functionality they use. We also have several constructors and functions where we have or are planning to go beyond what the ECMAScript standard stipulates.

The PR looks good and should be ready to merge soon; thanks for your work on this!

Aside: When applying suggestions from code review, it is possible to batch them all into a single commit when doing it from the files tab. That is potentially faster than accepting them and merging them one by one.

@marsian83
Copy link
Contributor Author

@Planeshifter Hi, apologies, I forgot to correct the licensing dates, I have signed and commited your suggestions.
Also, thanks for letting me know about the internal dependency convention,

@Planeshifter
Copy link
Member

@marsian83 Running CI and assuming everything passes, this should be ready to land!

Copy link
Member

@Planeshifter Planeshifter left a comment

Choose a reason for hiding this comment

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

Please have a look at the CI failures; for repl.txt, we want to ensure that we don't exceed 80 characters per line to display nicely in a terminal window. Should add a linebreak there where required.

Co-authored-by: Philipp Burckhardt <pburckhardt@outlook.com>
Signed-off-by: Spandan Barve <114365550+marsian83@users.noreply.github.com>
@marsian83
Copy link
Contributor Author

Hi, I signed off and commited your messages. Thanks for highlighting what I should keep in mind from next time on.

I had a question though...

// eslint-disable-line stdlib/no-redeclare

I did not understand what this comment is for and where should I use this. googling about this only results in "Disabling an eslint feature" but why and when would we want to do so?

marsian83 and others added 4 commits February 23, 2024 22:14
Co-authored-by: Philipp Burckhardt <pburckhardt@outlook.com>
Signed-off-by: Spandan Barve <114365550+marsian83@users.noreply.github.com>
Co-authored-by: Philipp Burckhardt <pburckhardt@outlook.com>
Signed-off-by: Spandan Barve <114365550+marsian83@users.noreply.github.com>
Co-authored-by: Philipp Burckhardt <pburckhardt@outlook.com>
Signed-off-by: Spandan Barve <114365550+marsian83@users.noreply.github.com>
Co-authored-by: Philipp Burckhardt <pburckhardt@outlook.com>
Signed-off-by: Spandan Barve <114365550+marsian83@users.noreply.github.com>
Co-authored-by: Philipp Burckhardt <pburckhardt@outlook.com>
Signed-off-by: Spandan Barve <114365550+marsian83@users.noreply.github.com>
@marsian83
Copy link
Contributor Author

I just noticed your edit regarding merging commits from the file section.
I will keep that in mind from next time on

Signed-off-by: Spandan Barve <114365550+marsian83@users.noreply.github.com>
…/index.js

Signed-off-by: Philipp Burckhardt <pburckhardt@outlook.com>
@Planeshifter
Copy link
Member

@marsian83 Will merge once CI passes.

Signed-off-by: Philipp Burckhardt <pburckhardt@outlook.com>
…k/benchmark.js

Signed-off-by: Philipp Burckhardt <pburckhardt@outlook.com>
@Planeshifter Planeshifter merged commit 50616dc into stdlib-js:develop Feb 23, 2024
7 checks passed
@marsian83 marsian83 deleted the patch-isnonnegativefinite branch February 23, 2024 19:35
kgryte added a commit that referenced this pull request Feb 24, 2024
kgryte added a commit that referenced this pull request Feb 24, 2024
kgryte added a commit that referenced this pull request Mar 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RFC]: add @stdlib/assert/is-nonnegative-finite
3 participants