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 math/base/special/acscd #1789

Merged
merged 10 commits into from
Mar 12, 2024
Merged

Conversation

the-r3aper7
Copy link
Contributor

@the-r3aper7 the-r3aper7 commented Mar 8, 2024

Resolves #42 .

Description

What is the purpose of this pull request?

This pull request:

  • Adds functionality to compute acscd in degree of any given input.
  • Say we have input x, so it should satisfies this x >= 1 or x <= -1 else it will return NaN

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

@Planeshifter Planeshifter added Needs Review A pull request which needs code review. Math Issue or pull request specific to math functionality. JavaScript Issue involves or relates to JavaScript. labels Mar 8, 2024
@Pranavchiku
Copy link
Member

Pranavchiku commented Mar 8, 2024

@the-r3aper7 we appreciate your contribution but opening 10 PRs in a span of 10 hour just don't make any sense. Also, it unnecessarily blocks other people to work on those. Neither of PR seems perfect to me, even description is not filled correctly. Reiterating we prefer quality of contribution over number of PRs.

@the-r3aper7
Copy link
Contributor Author

the-r3aper7 commented Mar 9, 2024

i am sorry, i will refactor them by today. i was just laying the foundation and I did not have intention to block other people work.

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.

@the-r3aper7 Left a few comments here upon cursory look through the package.

One important thing that should be added (here and in your other PRs) are tests showing that the function returns the correct values for valid inputs. You can generate a set of test values and add a test with description the function computes the arccosecant (in degrees) to test.js. Please have a look at other packages on how these are typically authored.

@the-r3aper7
Copy link
Contributor Author

the-r3aper7 commented Mar 9, 2024

cool i will make changes to other PRs too. About tests part i have intentionally left those because I wanted to ask how can I generate that huge json file which have inputs.

@the-r3aper7
Copy link
Contributor Author

@Planeshifter i have made those changes!

@the-r3aper7 the-r3aper7 changed the title feat: add math/base/special/acscd feat: add math/base/special/acscd Mar 9, 2024
@the-r3aper7
Copy link
Contributor Author

@Planeshifter i have made changes all the other PRs too. can you review those.

Signed-off-by: Sai Srikar Dumpeti <[email protected]>
Signed-off-by: Sai Srikar Dumpeti <[email protected]>
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.

This should be almost ready to merge now. Thanks for updating!

@the-r3aper7
Copy link
Contributor Author

cool i will make changes to other PRs. too

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.

Did one more look over the package; looks all good to me!

Will merge shortly.

@Planeshifter Planeshifter merged commit 1d7b726 into stdlib-js:develop Mar 12, 2024
7 checks passed
@the-r3aper7 the-r3aper7 deleted the acscd branch March 12, 2024 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JavaScript Issue involves or relates to JavaScript. Math Issue or pull request specific to math functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement acscd
3 participants