-
Notifications
You must be signed in to change notification settings - Fork 21
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
Disaggregated stats wrong total #191
base: main
Are you sure you want to change the base?
Conversation
Per discussion with @jnm, we may need to show stats for |
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.
Could you include a unit test with this PR?
src/formpack/schema/fields.py
Outdated
if val is None: | ||
not_provided += sum(counter.values()) | ||
else: | ||
# `counter['__submissions__'] contains the count of all submissions, |
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'm having trouble figuring out where this happens. I found these:
formpack/src/formpack/reporting/autoreport.py
Lines 161 to 162 in 09c600f
if value is not None: | |
counters['__submissions__'] += 1 |
formpack/src/formpack/reporting/autoreport.py
Lines 84 to 85 in 09c600f
counter.update(values) | |
counter['__submissions__'] += 1 |
...but they both increment
counter['__submissions__']
only when the value is not None
.
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.
@jnm,
Let's say, we have a form with two questions (select one), not mandatory:
Favorite coffee:
- Nespresso
- Keurig
- Tim Horton
Type of coffee:
- Regular
- Espresso
- Latte
Three users submit data (first case):
First one:
- Favorite coffee: Nespresso
- Type of coffee: Regular
Second one:
- Favorite coffee: Tim Horton
- Type of coffee: Latte
Third one:
- Favorite coffee: Keurig
- Type of coffee: No choices checked
Disaggregating the stats by grouping by Favorite coffee
should return 3 Counter
s
None
=>{'keuring': 1}
Latte
=>{'tim_horton': 1}
Regular
=>{'nespresso': 1}
Now, a fourth user submits this (second case):
- Favorite coffee: No choices checked
- Type of coffee: latte
It should still return the 3 sames counters, except that Latte
Counter should have changed for
{'None': 1, 'tim_horton': 1}
Having that said, the if
condition handles the first case, the else
handles the second one.
@noliveleger, sorry for abandoning this for so long. I'm trying once again to understand it, but I've found that the unit test passes without any of the other changes:
Here is the branch with the unit test commit cherry-picked on top of master: https://github.com/kobotoolbox/formpack/tree/cherry-pick-191-test. Did we implement a fix for #190 somewhere along the line after you opened this PR, or does the test need to be modified? |
@jnm, no. Unfortunately, my test is totally wrong. The issue is that |
@noliveleger what's the status of this PR? |
I've applied requested changes ;-) |
Fixes #190.