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

Make interval equality operator compare start and end of intervals #1136

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

jakemanger
Copy link

An approach to Fix #1135
This changes the default behaviour from comparing the interval duration to comparing the start and end of the interval, which I believe is more intuitive (it has slipped me up before). However, would this require a warning to notify users that the default behaviour has changed?

@vspinu
Copy link
Member

vspinu commented Oct 8, 2023

Hi @jakemanger, there is a failure in pkgdown. I think you need to declare it in the index.

More importantly, Should we add inequalities as well, for full partial order implementation?

Also could you please test for vectorized operations, not just single-tons, and add a new-features news entry?

Thanks!!!

@jakemanger
Copy link
Author

Hi @vspinu I've declared == and != in the index and have added inequalities and vector support. That vectors test was important, as I had used && previously where & is needed for vectors. I am running into an unusual issue with the tests of vctrs now. I haven't edited any part of that in the project. Do you know why this could be happening?

@vspinu
Copy link
Member

vspinu commented Dec 4, 2024

Hey @jakemanger sorry for the death ear for a year :( I was out of maintenance for a while. Indeed, the interval objects are actually vectorized objects, so the equality should be done element wise. The vctrz part is also important, but probably will be fixed once the operation is vectorized.

And fineally, it's a breaking change indeed. A carefull revdep has to be run before merging this one. If it impacts a lot of dependencies then we are in trouble.

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.

Equal to == operator doesn't work as expected with lubridate intervals
2 participants