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

Clarify interval comparison behavior with documentation and tests #5192

Merged
merged 2 commits into from
Dec 8, 2023

Conversation

alamb
Copy link
Contributor

@alamb alamb commented Dec 8, 2023

Which issue does this PR close?

Follow on to #5180

Rationale for this change

I was quite confused by the discussion on #5180. My final understanding is that the confusion stems from the fact that when different people "interval comparison" they may mean different things.

Given the subtleness of this topic and potential for confusion, I would like to avoid future confusion by better documenting the behavior of interval comparisons in the arrow crate so the intent is clear.

What changes are included in this PR?

  • Update documentation on how the different Interval types are compared and explaining the behavior of interval comparisons
  • Add some tests to ensure that the behavior of interval comparisons is as documented

Are there any user-facing changes?

More docs

@github-actions github-actions bot added the arrow Changes to the arrow crate label Dec 8, 2023
@alamb alamb force-pushed the alamb/interval_clarification branch from 3197fc4 to 4361eef Compare December 8, 2023 15:54
@alamb alamb mentioned this pull request Dec 8, 2023
@alamb alamb marked this pull request as ready for review December 8, 2023 15:58
Copy link
Contributor

@tustvold tustvold 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 this

@tustvold tustvold merged commit c5a9953 into apache:master Dec 8, 2023
26 checks passed
@alamb alamb self-assigned this Dec 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrow Changes to the arrow crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants