-
Notifications
You must be signed in to change notification settings - Fork 0
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
Feature: Add 5 new member activity analysis #13
base: main
Are you sure you want to change the base?
Conversation
- We needed to feed the new inconsistent type in analysis pipeline.
- all_new_consistent - all_new_vital - all_became_inconsistent - all_became_unvital
- all_new_consistent - all_new_vital - all_became_not_consistent - all_became_unvital also we fixed the linter issues related to test_inconsistently_active.py
"11": {"user0", "user1"}, | ||
"12": {"user0", "user1"}, | ||
"13": {"user0", "user1"}, | ||
"14": set(), # was incluedd in all_paused here |
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.
Is this comment correct? If the activity stops on day 14 I would expect it to be all_paused until day 20. Furthermore, all_paused should not exclude it being all_inconsistent. As a result, I'm not sure why day 14 is not counted as all_inconsistent.
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 think I've added this comment wrongly when I was copying and pasting some parts of the other tests to this test. So the results for this test is as blow
all_consistent: {'0': set(), '1': set(), '2': set(), '3': set(), '4': set(), '5': set(), '6': set(), '7': set(), '8': set(), '9': set(), '10': set(), '11': set(), '12': set(), '13': set(), '14': {'user0', 'user1'}, '15': set(), '16': set(), '17': set(), '18': set(), '19': set(), '20': set(), '21': {'user0', 'user1'}, '22': set(), '23': set(), '24': set(), '25': set(), '26': set(), '27': set()}
all_paused: {'0': set(), '1': set(), '2': set(), '3': set(), '4': set(), '5': set(), '6': set(), '7': set(), '8': set(), '9': set(), '10': set(), '11': set(), '12': set(), '13': set(), '14': set(), '15': {'user0', 'user1'}, '16': {'user0', 'user1'}, '17': {'user0', 'user1'}, '18': {'user0', 'user1'}, '19': {'user0', 'user1'}, '20': {'user0', 'user1'}, '21': {'user0', 'user1'}, '22': set(), '23': set(), '24': set(), '25': set(), '26': set(), '27': set()}
all_inconsistent: {'0': set(), '1': set(), '2': set(), '3': set(), '4': set(), '5': set(), '6': set(), '7': {'user0', 'user1'}, '8': {'user0', 'user1'}, '9': {'user0', 'user1'}, '10': {'user0', 'user1'}, '11': {'user0', 'user1'}, '12': {'user0', 'user1'}, '13': {'user0', 'user1'}, '14': set(), '15': {'user0', 'user1'}, '16': {'user0', 'user1'}, '17': {'user0', 'user1'}, '18': {'user0', 'user1'}, '19': {'user0', 'user1'}, '20': {'user0', 'user1'}, '21': set(), '22': set(), '23': set(), '24': set(), '25': set(), '26': set(), '27': set()}
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.
Everything looks good except for 1 question about a test. After that is sorted out, I'll approve
It still surprises me that it is not counted as inconsistent on the 14th. Can you confirm that the accounts were not active on the 14th? This is what I understand based on the code. However, if there was no activity on the 14th then they shouldn't be consistent and therefore they should be inconsistent. Can you confirm that the account is not consistent and that it is paused on the 14th day? |
Thanks for asking. So on day 14, the users are actually active and from day 15 day are not active. Having the users as (just reiterating on the discussion so we would remember)
and the answer was we have three consecutive weeks (0, 7, 14) that is making the users to be consistent and based on the inconsistent formula, they wouldn't be inconsistent. |
Ah great, then all looks good! |
We're adding 5 new activity type from the memberactivity analyzer output in this PR.
the activity types are