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

Add percentage-calculation functions #609

Merged
merged 1 commit into from
Oct 11, 2024
Merged

Conversation

sharkdp
Copy link
Owner

@sharkdp sharkdp commented Oct 9, 2024

>>> 72 € |> increase_by(15%)

    = 82.8 €    [Money]

>>> percentage_change(3.2 kg, 4.7 kg)

    = 46.875 %

Additional ideas / TODOs:

  • Special syntax for increase_by/decrease_by?
  • Special/custom error message when attempting to do something like 72 € - 15%.

closes #494

@sharkdp sharkdp mentioned this pull request Oct 9, 2024
@Goju-Ryu
Copy link
Contributor

Goju-Ryu commented Oct 9, 2024

This looks great, but the name of percentage_chance is a bit confusing to me. I had to reread some examples to figure out what the intent was.

Maybe something like percentage_difference would be more clear?

So the current:

assert_eq(percentage_chance(40 dollar, 50 dollar), +25%)

Would be this instead:

assert_eq(percentage_difference(50 dollar, 40 dollar), +25%)
# or with pipeline 
assert_eq((40 dollar |> percentage_difference(50 dollar)), +25%)

I’m not sure if it is exactly right, but I think naming it something without the word chance would make it more easily understood.

@sharkdp
Copy link
Owner Author

sharkdp commented Oct 9, 2024

percentage_chance

without the word chance

Is this a typo or did you misread? The function is called "percentage_change".

@Goju-Ryu
Copy link
Contributor

Goju-Ryu commented Oct 9, 2024

Is this a typo or did you misread? The function is called "percentage_change".

It wasn't a typo, I see now that I misread it multiple times over. It makes a lot more sense with the naming now that I actually read it right.

@andylima
Copy link

Thank you for adding this. 🙂 It's a bit verbose though, so I'm looking forward to that special syntax for increase_by / decrease_by — What do you have in mind?

@sharkdp
Copy link
Owner Author

sharkdp commented Oct 11, 2024

Thank you for adding this. 🙂 It's a bit verbose though, so I'm looking forward to that special syntax for increase_by / decrease_by — What do you have in mind?

I like those -%/+% operators that you proposed in #494. Maybe we could open a new ticket for those additional tasks? We can merge this as-is, I think.

@sharkdp sharkdp merged commit 5e8e63a into master Oct 11, 2024
15 checks passed
@sharkdp sharkdp deleted the percentage-calculations branch October 11, 2024 11:06
@andylima
Copy link

I like those -%/+% operators that you proposed in #494. Maybe we could open a new ticket for those additional tasks? We can merge this as-is, I think.

That's great! 🙂 Do you want me to open the new ticket?

@sharkdp
Copy link
Owner Author

sharkdp commented Oct 11, 2024

That'd be great — thanks.

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.

1000 - 20%
3 participants