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 compare function #12

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

justincy
Copy link
Contributor

Regarding #3. Works for simple and approximate dates. Considers partial dates, +1940, to equal more specified dates, +1940-12-21, it the defined parts match.

@justincy
Copy link
Contributor Author

This can't support partial dates. As is, +1940 = +1940-02-17 and +1940 = +1940-10-04 but +1940-02-17 < +1940-10-04. In other words, we have A = B and A = C but B != C. This leads to a non-deterministic sort order (it depends on the order of the input) which is broken.

@justincy
Copy link
Contributor Author

I think the best solution is to require that the dates have the same parts defined. That way we could sort a list of years, a list of year/months, a list of days, or a list of date times, but not a mixed list.

@justincy
Copy link
Contributor Author

There are alternatives.

We could say that +1940 is always less than any more specific date in 1904. That would almost be akin to adding missing months with 01 and missing days with 01, the exception being that in this case +1940 would be less than +1940-01-01 as opposed to equal.

Or we could do the opposite and say +1940 is always greater than more specific dates.

Or we could try something in the middle like July 1st.

Either way we would be making some assumptions, and it's better for the user to do that instead of us. If they want to handle partial dates, let them be the ones who decide how to force them into a state that will behave deterministically.

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.

1 participant