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

Fix corner cases of cut with duplicated breaks #410

Merged
merged 7 commits into from
Dec 30, 2024
Merged

Fix corner cases of cut with duplicated breaks #410

merged 7 commits into from
Dec 30, 2024

Conversation

nalimilan
Copy link
Member

@nalimilan nalimilan commented Dec 27, 2024

Apply more systematically the rule that all intervals are closed on the right left and open on the left right except the last one. Throw an error when this would lead to empty intervals unless allowempty=true. Handle 0.0 more systematically.

Fixes #382.

I'm not sure what's the best way of handling -0.0. The current state of the PR treats it as different from 0.0, since that's consistent with our using isless (via searchsortedlast). But we could probably implement a different behavior if we wanted to. 0.0 can be confusing, but if users have it in their data they may wish to preserve it.

I should also check for NaN and throw an error, which is what happened before this PR though it's undocumented and untested.

Apply more systematically the rule that all intervals are closed on the right
and open on the left except the last one. Throw an error when this would lead
to empty intervals unless `allowempty=true`.
@nalimilan nalimilan added this to the 1.0 milestone Dec 27, 2024
@nalimilan nalimilan requested a review from bkamins December 27, 2024 22:09
@nalimilan nalimilan marked this pull request as ready for review December 28, 2024 14:32
@bkamins
Copy link
Member

bkamins commented Dec 28, 2024

Apply more systematically the rule that all intervals are closed on the right and open on the left except the last one.

It is the opposite. Intervals are closed on the left and open on the right (except the last one). But it is OK.

@nalimilan
Copy link
Member Author

Let's check whether Nanosoldier works on two packages:
@nanosoldier runtests(["DataFrames", "MLJ"])

@nanosoldier
Copy link

Your job submission was not accepted. Consult the server logs for more details (cc @maleadt).

@JuliaData JuliaData deleted a comment from nanosoldier Dec 29, 2024
@maleadt
Copy link

maleadt commented Dec 30, 2024

Should work now:

@nanosoldier runtests(["DataFrames", "MLJ"])

@nanosoldier
Copy link

Update on PkgEvalJob cc547a3 vs. bbac8dc: Accepted

@nanosoldier
Copy link

Update on PkgEvalJob cc547a3 vs. bbac8dc: Running

@nanosoldier
Copy link

The package evaluation job you requested has completed - no new issues were detected.
The full report is available.

Number of groups was sometimes incorrect.
@nalimilan nalimilan merged commit 341de70 into master Dec 30, 2024
14 of 16 checks passed
@nalimilan nalimilan deleted the nl/cut2 branch December 30, 2024 21:47
@nalimilan nalimilan mentioned this pull request Dec 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Strange cut for imbalanced distributions
4 participants