-
Notifications
You must be signed in to change notification settings - Fork 52
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/no answer pipeline #183
base: main
Are you sure you want to change the base?
Feature/no answer pipeline #183
Conversation
This is a good start! But it's missing a key feature, which is that the What I mean is that currently this gives up data like this (in this case, "years of experience" with the "gender" facet):
So you've added the "years of experience" breakdown for people who didn't answer the "gender" question. But within each "years of experience" array of buckets, we also want to know how many people didn't answer the years of experience question. So the data we actually want for would be more like this:
Additionally we want this
|
api/src/data/keys.yml
Outdated
@@ -9,6 +9,7 @@ age: | |||
- range_more_than_65 | |||
|
|||
years_of_experience: | |||
- no_answer |
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.
Because every field will need a no_answer
key I don't think it makes sense to add it here, it's probably better to add it directly in generic.ts
somewhere.
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 not sure we actually need to add it though, or maybe only at the GraphQL level… at least I don't think we use those keys in the pipeline? I'll double check.
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.
Previously
This is a good start! But it's missing a key feature, which is that the
no_answer
key should be added to buckets, not just facets.
Generic pipeline updated. Added "no_answer" key to the buckets section. Also, when no facets is selected, I tested and got results as you wrote before.
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.
Because every field will need a
no_answer
key I don't think it makes sense to add it here, it's probably better to add it directly ingeneric.ts
somewhere.
Yes, I absolutely agree. If 'no_answer' key is needed for every field, better to add it in generic.ts or at the GraphQL level (if keys.yml are not used in pipeline). I have checked - don't found straight use of keys in Mongo aggregation, but you better check.
api/src/compute/generic_pipeline.ts
Outdated
// $unwind: { | ||
// path: `$${facetPath}` | ||
// } | ||
// } |
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 guess we can safely remove this whole step then?
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.
Removed comments. Changed aggregation.
By the way, that (Also I guess it wouldn't be too hard to do it outside the aggregation pipeline in the rest of the JS code if the pipeline can't easily do it) |
Good progress! But now I'm running into a different issue. It doesn't work when querying for a field where people can pick multiple options at the same time. For example with the following GraphQL query:
I get this:
As you can see it's using every existing combination of answers as a unique
|
… multi select option
…norepo into feature/no-answer-pipeline
Someone is attempting to deploy a commit to the Devographics Team on Vercel. A member of the Team first needs to authorize it. |
I have added back unwind operator with specific option which not skip nullable/empty fields. Seems, that we cannot remove unwind operator. Tested your case, working fine now, tested previous cases locally also - seems working for me. |
Added new noAnswer key and updated generic pipeline aggregation to show all responses without answer.