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

Add csc, sec, cot, and friends #50

Merged
merged 1 commit into from
Jul 23, 2022
Merged

Add csc, sec, cot, and friends #50

merged 1 commit into from
Jul 23, 2022

Conversation

triallax
Copy link
Contributor

For sharkdp/insect#212 (comment). I also added some other functions.

What shall I do for tests?

triallax
triallax commented Mar 26, 2022
View reviewed changes
src/Quantities.purs Outdated Show resolved Hide resolved
@sharkdp
Copy link
Owner

sharkdp commented Mar 29, 2022

Thank you for the updates. The roundtrip-style tests (f_inverse(f(x)) == x && f(f_inverse(y)) == y) sound like a great idea!

@triallax triallax marked this pull request as draft April 1, 2022 15:54
@triallax
Copy link
Contributor Author

triallax commented Apr 2, 2022

I think I'm going to have to stop my work for now, since real life is catching up with me. I won't be doing any work for around a month and a half, so feel free to pick up this PR and complete it, or wait for me till I come back. Your call. :)

@sharkdp
Copy link
Owner

sharkdp commented Apr 3, 2022

Of course. There's never any rush on anything, please take your time. And yes, otherwise someone else can pick it up in the meantime.

triallax
triallax commented Jun 25, 2022
View reviewed changes
test/Main.purs Outdated Show resolved Hide resolved
triallax
triallax commented Jun 27, 2022
View reviewed changes
test/Main.purs Outdated Show resolved Hide resolved
@triallax triallax force-pushed the add-sec-and-friends branch 3 times, most recently from 2ce20e1 to 1fa4855 Compare June 27, 2022 09:21
@triallax
Copy link
Contributor Author

I guess having some individual function tests with negative and positive arguments (as you mentoined before) would also be a good idea?

@sharkdp
Copy link
Owner

sharkdp commented Jun 27, 2022

I guess having some individual function tests with negative and positive arguments (as you mentoined before) would also be a good idea?

Maybe at least for the functions with multiple (explicit) branches in the implementation (acot), that would be great.

@triallax
Copy link
Contributor Author

I just realized there's no reason not to have these functions in purescript-decimals, what do you think about putting them there and then just wrapping them here?

@sharkdp
Copy link
Owner

sharkdp commented Jun 28, 2022 •
edited
Loading

I personally don't depend on purescript-decimals in any other project, so I'm also fine with leaving them here. But it makes a lot of sense, of course. Unless we consider purescript-decimals to be just a thin wrapper around Decimal.js. Which is also a reasonable point of view, IMO.

@triallax
Copy link
Contributor Author

triallax commented Jun 29, 2022 •
edited
Loading

I personally don't depend on purescript-decimals in any other project, so I'm also fine with leaving them here.

Yeah, but I can imagine it might be useful for other people.

Unless we consider purescript-decimals to be just a thin wrapper around Decimal.js. Which is also a reasonable point of view, IMO.

It is definitely a reasonable point of view, but I'm leaning toward the earlier opinion. What about you?

@sharkdp
Copy link
Owner

sharkdp commented Jul 1, 2022

Ok, fine with me!

@triallax triallax mentioned this pull request Jul 1, 2022
Merged
@triallax triallax force-pushed the add-sec-and-friends branch 2 times, most recently from 901bb1d to b84d049 Compare July 19, 2022 20:55
@triallax
Add csc, sec, cot, and friends
a01b6b4 
The package set was upgraded in this commit to get these functions in
the new `purescript-decimals` version, and PureScript was upgraded with
the package set accordingly.
@triallax triallax marked this pull request as ready for review July 20, 2022 22:34
sharkdp
sharkdp approved these changes Jul 23, 2022
View reviewed changes
Copy link
Owner

@sharkdp sharkdp left a comment

Choose a reason for hiding this comment

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

Thank you for the update.

@sharkdp sharkdp merged commit bdeaa40 into sharkdp:master Jul 23, 2022
@triallax triallax deleted the add-sec-and-friends branch July 24, 2022 11:35
@triallax
Copy link
Contributor Author

@sharkdp can you cut a new release 12.1.0/13.0.0 (not sure which one)?

maybe I should learn how to create releases myself

@sharkdp
Copy link
Owner

sharkdp commented Jul 25, 2022 •
edited
Loading

@sharkdp can you cut a new release 12.1.0/13.0.0 (not sure which one)?

maybe I should learn how to create releases myself

Feel free to try! And let me know if I need to set up any permissions.

So far, I simply set a Git tag and then called pulp publish.

@triallax
Copy link
Contributor Author

So I went ahead and followed https://github.com/purescript/spago#publish-my-library, and I think I've done everything correctly.

@sharkdp
Copy link
Owner

sharkdp commented Jul 25, 2022

Thank you!

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

@sharkdp sharkdp sharkdp approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@triallax @sharkdp