-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
[ASM] - Expandr 4735 #27624
[ASM] - Expandr 4735 #27624
Changes from 3 commits
bdf9d3d
278df10
7b2f63f
ad3c0ed
59a81b1
941af29
8e9c40b
c984006
5d6c938
b5f2548
4ded770
d0023c3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
|
||
#### Scripts | ||
|
||
##### RankServiceOwners | ||
|
||
- Updated the script to return a high-confidence set of most likely owners based on their relative ranking scores. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,12 @@ | ||
import demistomock as demisto # noqa: F401 | ||
import pytest | ||
import unittest | ||
from RankServiceOwners import score, main, rank, _canonicalize, aggregate | ||
from RankServiceOwners import score, main, rank, _canonicalize, aggregate, _get_k | ||
|
||
|
||
@pytest.mark.parametrize('owners,expected_out', [ | ||
( | ||
# returned in sorted order | ||
[ | ||
{ | ||
'Name': 'bob', 'Email': 'bob@example.com', 'Source': '', | ||
|
@@ -27,6 +28,61 @@ | |
}, | ||
] | ||
), | ||
( | ||
# wraps one test case from _get_k | ||
[ | ||
{ | ||
'Name': 'a', 'Email': 'a@example.com', 'Source': '', | ||
'Timestamp': '', 'Ranking Score': 10, 'Justification': '' | ||
}, | ||
{ | ||
'Name': 'b', 'Email': 'b@example.com', 'Source': '', | ||
'Timestamp': '', 'Ranking Score': 1, 'Justification': '' | ||
}, | ||
{ | ||
'Name': 'c', 'Email': 'c@example.com', 'Source': '', | ||
'Timestamp': '', 'Ranking Score': 1, 'Justification': '' | ||
}, | ||
{ | ||
'Name': 'd', 'Email': 'd@example.com', 'Source': '', | ||
'Timestamp': '', 'Ranking Score': 1, 'Justification': '' | ||
}, | ||
{ | ||
'Name': 'e', 'Email': 'e@example.com', 'Source': '', | ||
'Timestamp': '', 'Ranking Score': 1, 'Justification': '' | ||
}, | ||
{ | ||
'Name': 'f', 'Email': 'f@example.com', 'Source': '', | ||
'Timestamp': '', 'Ranking Score': 1, 'Justification': '' | ||
}, | ||
], | ||
[ | ||
{ | ||
'Name': 'a', 'Email': 'a@example.com', 'Source': '', | ||
'Timestamp': '', 'Ranking Score': 10, 'Justification': '' | ||
}, | ||
{ | ||
'Name': 'b', 'Email': 'b@example.com', 'Source': '', | ||
'Timestamp': '', 'Ranking Score': 1, 'Justification': '' | ||
}, | ||
{ | ||
'Name': 'c', 'Email': 'c@example.com', 'Source': '', | ||
'Timestamp': '', 'Ranking Score': 1, 'Justification': '' | ||
}, | ||
{ | ||
'Name': 'd', 'Email': 'd@example.com', 'Source': '', | ||
'Timestamp': '', 'Ranking Score': 1, 'Justification': '' | ||
}, | ||
{ | ||
'Name': 'e', 'Email': 'e@example.com', 'Source': '', | ||
'Timestamp': '', 'Ranking Score': 1, 'Justification': '' | ||
}, | ||
{ | ||
'Name': 'f', 'Email': 'f@example.com', 'Source': '', | ||
'Timestamp': '', 'Ranking Score': 1, 'Justification': '' | ||
}, | ||
] | ||
), | ||
]) | ||
def test_rank(owners, expected_out): | ||
assert rank(owners) == expected_out | ||
|
@@ -395,3 +451,56 @@ def test_main(mocker, owners, expected_out, capfd): | |
# Verify the output value was set | ||
expected_calls_to_mock_object = [unittest.mock.call('setAlert', {'asmserviceowner': expected_out})] | ||
assert demisto_execution_mock.call_args_list == expected_calls_to_mock_object | ||
|
||
|
||
def test_get_k(): | ||
""" | ||
These cases are designed to specify the intuition we are trying to implement with the algorithm. | ||
They are specific to a target value of 5; if the target_k changes, these tests should update to reflect that. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The docstring should be generic, so mentioning 5 is not appropriate here unless you're actually setting that parameter in _get_k -- instead you might mention that this test verifies the default case. (The place to note "5" is in the docstring for _get_k -- there it is worth flagging that all the default values are tuned for k=5.) |
||
We assert that if the algorithm matches our intuition at least 80% of the time, it's probably fine. | ||
|
||
See function documentation for explanation of hyperparameters and their defaults. | ||
""" | ||
|
||
# The first value in each cases is the list of scores output by the model (one per owner) | ||
# and the second value is the expected k | ||
cases = [ | ||
# If smallish set of owners, return all or find obvious cutoff | ||
([1], 1), | ||
([1, 1], 2), | ||
([1, 1, 1], 3), | ||
([10, 1, 1], 3), | ||
([1, 1, 1, 1], 4), | ||
([10, 1, 1, 1], 4), | ||
([10, 10, 1, 1], 4), # or 2; either seems fine | ||
([10, 10, 1, 1], 2), # or 4; either seems fine | ||
([1, 1, 1, 1, 1], 5), | ||
([10, 1, 1, 1, 1], 5), | ||
([10, 10, 1, 1, 1], 2), | ||
([1, 1, 1, 1, 1, 1], 6), | ||
([1, 1, 1, 1, 1, 1, 1], 7), | ||
|
||
# If larger set of owners, return top handful or find obvious cutoff | ||
([1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1], 5), | ||
([10, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1], 5), | ||
([10, 10, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1], 5), # or 2; either seems fine | ||
([10, 10, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1], 2), # or 5; either seems fine | ||
([10, 10, 10, 1, 1, 1, 1, 1, 1, 1, 1, 1], 3), | ||
([10, 10, 10, 10, 1, 1, 1, 1, 1, 1, 1, 1], 4), | ||
([10, 10, 10, 10, 10, 1, 1, 1, 1, 1, 1, 1], 5), | ||
([100, 10, 10, 10, 10, 1, 1, 1, 1, 1, 1, 1], 5), | ||
([100, 10, 10, 10, 10, 10, 1, 1, 1, 1, 1, 1], 6), | ||
|
||
# Do something reasonable for non-obvious cutoffs | ||
([10, 9, 8, 7, 6, 5, 4, 3, 2, 1], 5), | ||
([19, 17, 15, 13, 11, 9, 7, 5, 3, 1], 5), | ||
|
||
# Do something reasonable for larger scales | ||
([500, 200, 100, 50, 25, 10, 1], 3), | ||
] | ||
num_correct = 0 | ||
for scores, expected_k in cases: | ||
if _get_k(scores) == expected_k: | ||
num_correct += 1 | ||
|
||
assert (num_correct / len(cases)) >= 0.8 |
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.
can you defensively verify that these values are within expected bounds? (for instance, that k_tol isn't -3.)