Conversation
|
Per discussion with @jnm, we may need to show stats for |
jnm
left a comment
There was a problem hiding this comment.
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.
I'm having trouble figuring out where this happens. I found these:
formpack/src/formpack/reporting/autoreport.py
Lines 161 to 162 in 09c600f
formpack/src/formpack/reporting/autoreport.py
Lines 84 to 85 in 09c600f
...but they both increment
counter['__submissions__'] only when the value is not None.
There was a problem hiding this comment.
@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 Counters
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.