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

Add GSR detailed analytics and fitness analytics #306

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

jonathanmelitski
Copy link

Prep for Wrapped. Added:

  1. Analytics for GSRs (instead of only one key, there's now a key that contains the location, room, and starttime + duration)
  2. Analytics for Fitness (added to the fetch data function. Eventually will be used to calculate "Total number of basketball hours this year" or other stats like that which seem interesting, I guess.)

Written schema for these new analytics keys will be developed once more are thought up.

@@ -0,0 +1,23 @@
from enum import Enum
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wasn't sure if it was good practice to use the pennmobile analytics.py file for penndata analytics calls (though they're essentially the same file.)

@vcai122
Copy link
Contributor

vcai122 commented Sep 5, 2024

Thanks for the PR! Always happy to have people working across codebases haha

I think what Justin had put up initially was just the initial sketch of what it analytics might look like and still needs a bit of work to be mature, and we had talked about a couple ideas before he left.

I think a solution like this where we are just adding code to places we need it will lead to unnecessary bloat, and less readability in general / harder to maintain, but the larger issue is other products will have to duplicate this code.

Probably we should make a more generalizable API that is shared across all product (with support for single data/multi-data transactions + key/value pairs) through Django-Labs-Accounts and then probably make it easy to use through the decorators is my current thoughts.

Let's discuss more on Friday

@JeffersonDing
Copy link
Member

Thanks for the PR! Always happy to have people working across codebases haha

I think what Justin had put up initially was just the initial sketch of what it analytics might look like and still needs a bit of work to be mature, and we had talked about a couple ideas before he left.

I think a solution like this where we are just adding code to places we need it will lead to unnecessary bloat, and less readability in general / harder to maintain, but the larger issue is other products will have to duplicate this code.

Probably we should make a more generalizable API that is shared across all product (with support for single data/multi-data transactions + key/value pairs) through Django-Labs-Accounts and then probably make it easy to use through the decorators is my current thoughts.

Let's discuss more on Friday

I agree. Jon and I were just thinking we should probably hack together something asap to get data into the DB so that we can push out wrapped end of this semester. Over the semester, there's a lot of optimizations we should do to the engine. I've been working on a couple on the lab-analytics repo and also DLA. Lets discuss Friday.

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

Successfully merging this pull request may close these issues.

3 participants