Skip to content
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

Expose MajorityLocation #4583

Closed
Tracked by #4578
jc-harrison opened this issue Nov 22, 2021 · 1 comment · Fixed by #4616
Closed
Tracked by #4578

Expose MajorityLocation #4583

jc-harrison opened this issue Nov 22, 2021 · 1 comment · Fixed by #4616
Assignees
Labels
FlowClient Issues related to FlowClient FlowMachine Issues related to FlowMachine

Comments

@jc-harrison
Copy link
Member

jc-harrison commented Nov 22, 2021

Expose MajorityLocation through the API, and add a corresponding flowclient query spec. This should be added to ReferenceLocationSchema and InputToFlowsSchema (while we're here, I'm wondering whether there's any reason not to add ReferenceLocationSchema to InputToFlowsSchema, to avoid duplication here).

The 'subscriber_location_weights' parameter should accept any appropriate exposed query (i.e. those with subscriber, location and numeric 'value' columns) - for the moment, I think this will only be LocationVisits.

I think 'weight_column' should be hard-coded to 'value', which will require renaming the 'dl_count' column of LocationVisits.

@jc-harrison jc-harrison mentioned this issue Nov 22, 2021
5 tasks
@jc-harrison jc-harrison added FlowClient Issues related to FlowClient FlowMachine Issues related to FlowMachine P-Now labels Nov 22, 2021
@jc-harrison
Copy link
Member Author

While we're addressing this, we should move MajorityLocation from 'features/location' to 'features/subscriber' - I failed to catch that one in review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FlowClient Issues related to FlowClient FlowMachine Issues related to FlowMachine
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants