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

Bugfix: Update negation syntax in compute_ground_truth_statistics #10

Merged
merged 2 commits into from
May 24, 2024

Conversation

CallumWSimprints
Copy link
Contributor

Hi,

First of all, amazing library - great work! ✨

I've spotted a potential bug surrounding negation in numpy. The ('-') syntax does not work for reversing a boolean array, which seems to get passed to the compute_ground_truth_statistics in certain scenarios (e.g., sometimes, but not always, when asking for the fast delong solver).

I've updated the code to use the tilde sign ('~'). I hope this small change helps as it works for both integer and boolean arrays.

Error

image

Minimal reproducible example & test case:

from confidenceinterval.delong import compute_ground_truth_statistics
import numpy as np
import pytest

@pytest.mark.parametrize(
    'ground_truth',
    [
        (np.array([0,0,1,1,1])), # Values are integers
        (np.array([False, False, True, True, True])) # values are bools
        ]
)
def test_compute_ground_truth_statistics_bool(ground_truth):
    
    sample_weight = np.array([1,1,1,1,1])
    
    expected_order = np.array([2,3,4,0,1])
    expected_label_1_count = 3
    expected_ordered_sample_weight = np.array([1,1,1,1,1])
    
    order, label_1_count, ordered_sample_weight = compute_ground_truth_statistics(ground_truth=ground_truth, sample_weight=sample_weight)
    
    assert np.array_equal(expected_order, order)
    assert expected_label_1_count == label_1_count
    assert np.array_equal(expected_ordered_sample_weight, ordered_sample_weight)

I can add this test to the library if you can let me know where it should go 😄 e.g., a new file called test_utils.py, or similar?

Keep up the good work,
Callum

@jacobgil
Copy link
Owner

Thanks so much! For the PR, and for the kind words.

I think test_delong.py would be great!

@CallumWSimprints
Copy link
Contributor Author

Thank you for the lightning-fast reply!

Test added in a new file called test_delong.py, ready for your review @jacobgil.

@jacobgil jacobgil merged commit 5dd3e58 into jacobgil:main May 24, 2024
1 check passed
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.

2 participants