-
Notifications
You must be signed in to change notification settings - Fork 38
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 enforcement for np.sort
and np.argsort
#918
Conversation
for more information, see https://pre-commit.ci
…rax into set_default_as_mergesort
for more information, see https://pre-commit.ci
…rax into set_default_as_mergesort
for more information, see https://pre-commit.ci
Unfortunately, the For So we have two options:
I'm checking the possibility for both ways. |
…on for highest_density_region
…rax into set_default_as_mergesort
for more information, see https://pre-commit.ci
…rax into set_default_as_mergesort
np.sort
and np.argsort
…rax into set_default_as_mergesort
for more information, see https://pre-commit.ci
…rax into set_default_as_mergesort
for more information, see https://pre-commit.ci
…rax into set_default_as_mergesort
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should mention that this PR is following up XENONnT/straxen#1176, for future reference. |
This PR enforces deterministic sorting behavior by implementing the
sort_enforcement
module. Specifically, it:mergesort
the default sorting algorithmquicksort
andheapsort
) to prevent their usagenp.sort
andnp.argsort
to enforce the behaviors mentioned aboveThe implementation is robust to import order - the enforcement takes effect whenever
strax
is imported, regardless of whethernumpy
is imported before or after. This ensures consistent sorting behavior across all strax operations, addressing the non-deterministic sorting issues reported in #916.A unit test is also added accordingly.