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 Duration.min() and Duration.max() #1527

Open
jirutka opened this issue Oct 10, 2023 · 4 comments
Open

Add Duration.min() and Duration.max() #1527

jirutka opened this issue Oct 10, 2023 · 4 comments

Comments

@jirutka
Copy link

jirutka commented Oct 10, 2023

Can you please add static methods min and max to the Duration, analogous to the methods in DateTime?

@shashankbhat10
Copy link

Hey @icambron,

If you are ok with adding these methods to the Duration class, can I be assigned this issue?
I have made some changes regarding this and can raise a PR for it.

@diesieben07
Copy link
Collaborator

This definitely is not trivial. The problem is that durations by default use "casual" conversion rules, which can make it so this comparison would not be reflexive or transitive, depending on which unit you use for comparison.
For example:
29 days > 4 weeks, using days
4 weeks == 1 month, using weeks
29 days < 1 month, using days

So, 29 days is both larger and smaller than 1 month, considering the above logic.

Also:
4 weeks == 1 month, using weeks; but
4 weeks < 1 month, using days (because 28 days < 30 days)

This gets even more complicated if you have multiple (even different) units on both sides. So really, the user needs to tell which unit to use for the comparison. And even then they might not get the result they want.

@shashankbhat10
Copy link

I get what you are saying. I think could be done after any resolution of #1514, right?

@diesieben07
Copy link
Collaborator

No, this is an inherent property of the casual conversion matrix that Durations use by default. You can read more about it in the documentation.

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

3 participants