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

feat: add support for secant functionality math/base/special/sec #2920

Closed
wants to merge 0 commits into from
Closed

feat: add support for secant functionality math/base/special/sec #2920

wants to merge 0 commits into from

Conversation

AbhijitRaut04
Copy link
Contributor

@AbhijitRaut04 AbhijitRaut04 commented Sep 18, 2024 •
edited
Loading

Resolves #225

Add the secant functionality using the existing cosine and sine functions.

The PR fullfills the purpose of calculating secant of a number.

This pull request:

  • is able to calculate secant of a number.

This pull request:

Questions

What modifications should I make for this PR ?

Checklist

@stdlib-js/reviewers

@kgryte
@Planeshifter

@stdlib-bot stdlib-bot added the Math Issue or pull request specific to math functionality. label Sep 18, 2024
@stdlib-bot
Copy link
Contributor

Hello! Thank you for your contribution to stdlib.

We noticed that the contributing guidelines acknowledgment is missing from your pull request. Here's what you need to do:

  1. Please read our contributing guidelines.

  2. Update your pull request description to include this checked box:

    - [x] Read, understood, and followed the [contributing guidelines](https://github.com/stdlib-js/stdlib/blob/develop/CONTRIBUTING.md)

This acknowledgment confirms that you've read the guidelines, which include:

  • The developer's certificate of origin
  • Your agreement to license your contributions under the project's terms

We can't review or accept contributions without this acknowledgment.

Thank you for your understanding and cooperation. We look forward to reviewing your contribution!

stdlib-bot
stdlib-bot reviewed Sep 18, 2024
View reviewed changes
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. 🏃 💨

@AbhijitRaut04 AbhijitRaut04 changed the title Addsupport for Secant functionality Add support for Secant functionality Sep 19, 2024
@AbhijitRaut04 AbhijitRaut04 changed the title Add support for Secant functionality feat: add support for secant functionality lib/node_modules/@stdlib/math/base/special/sec Sep 19, 2024
@AbhijitRaut04 AbhijitRaut04 changed the title feat: add support for secant functionality lib/node_modules/@stdlib/math/base/special/sec feat: add support for secant functionality Sep 19, 2024
@AbhijitRaut04 AbhijitRaut04 changed the title feat: add support for secant functionality feat: add support for secant functionality lib/node_modules/@stdlib/math/base/special/sec Sep 19, 2024
@Planeshifter Planeshifter changed the title feat: add support for secant functionality lib/node_modules/@stdlib/math/base/special/sec feat: add support for secant functionality math/base/special/sec Sep 21, 2024
@Planeshifter Planeshifter requested review from Planeshifter and removed request for headlessNode September 21, 2024 19:28
@kgryte kgryte added Needs Review A pull request which needs code review. Feature Issue or pull request for adding a new feature. labels Oct 1, 2024
@AbhijitRaut04
Copy link
Contributor Author

@kgryte Are there any changes required?

kgryte
kgryte reviewed Oct 2, 2024
View reviewed changes
lib/node_modules/@stdlib/math/base/special/sec/src/main.c Outdated
*
* ## Notice
*
* The following copyright, license, and long comment were part of the original implementation available as part of [FreeBSD]{@link https://svnweb.freebsd.org/base/release/12.2.0/lib/msun/src/s_tan.c}. The implementation follows the original, but has been modified for JavaScript.
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't actually point to the original implementation. This link is for kernel tan, not sec.

@kgryte
Copy link
Member

kgryte commented Oct 2, 2024

@AbhijitRaut04 I have no idea where you got the implementation. It looks like you copied from some other package. The implementation for this package should be trivial and is explicitly provided in #225.

As such, this package needs several changes.

@kgryte kgryte added Needs Changes Pull request which needs changes before being merged. and removed Needs Review A pull request which needs code review. labels Oct 2, 2024
@kgryte
Copy link
Member

kgryte commented Oct 2, 2024

Furthermore, given that tests and benchmarks are failing, it seems evident that you have not setup your local dev environment for linting, running tests, and running benchmarks. Please consult our contributing guide and our development guide.

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

@headlessNode headlessNode headlessNode left review comments

@kgryte kgryte kgryte left review comments

@stdlib-bot stdlib-bot Awaiting requested review from stdlib-bot

@Planeshifter Planeshifter Awaiting requested review from Planeshifter

Assignees
No one assigned
Labels
Feature Issue or pull request for adding a new feature. Math Issue or pull request specific to math functionality. Needs Changes Pull request which needs changes before being merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[RFC]: add support for computing the secant (sec)
4 participants
@AbhijitRaut04 @stdlib-bot @kgryte @headlessNode