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

I'm wondering if this should have been >= instead of > . The min_value_and_waste change policy was previously returning change for excess equal to the min value. #15

Open
LLFourn opened this issue Jan 3, 2024 · 3 comments

Comments

@LLFourn
Copy link
Collaborator

LLFourn commented Jan 3, 2024

          I'm wondering if this should have been `>=` instead of `>` . The `min_value_and_waste` change policy was previously returning change for excess equal to the min value.

Originally posted by @jp1ac4 in #14 (comment)

Create a test for this.

@jp1ac4
Copy link
Contributor

jp1ac4 commented Jan 10, 2024

For this test, should it also check that if the change policy min value is 0 and the drain value is 0, then drain_value still returns None (i.e. we don't include a drain with zero value)?

EDIT: Perhaps it would be simpler to keep it as > and let the user set the min value accordingly.

@LLFourn
Copy link
Collaborator Author

LLFourn commented Jan 22, 2024

Well if we keep it as > we have to change the name since I read min as inclusive.

@jp1ac4
Copy link
Contributor

jp1ac4 commented Jul 9, 2024

Well if we keep it as > we have to change the name since I read min as inclusive.

I agree, I think it's better to change it back to >=. I can try working on this if you like.

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

No branches or pull requests

2 participants