-
Notifications
You must be signed in to change notification settings - Fork 3
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
Filter_loaded_trips #46
Filter_loaded_trips #46
Conversation
allenmichael099
commented
Jun 17, 2021
- made mod_load_trips
- queried trips by timestamp
- made a new summarise_trips that doesn't require all of trips data
- made mod_load_trips - queried trips by timestamp - made a new summarise_trips that doesn't require all of trips data
- set the map to use trips_with_trajectories - uncommented locations and trajectories within mod_load_trips - locations are now filtered by the same dates as trips
- set max trip date window to 31 days - worked on handling empty query but no luck yet
- added functionality to prevent the user from getting an empty query - the checks are done in mod_load_trips in the load_allowed reactive - removed remaining traces of trips from mod_load_data
There’s more to improve after I get back but I think this is good to go for the moment. |
@asiripanich I am going to try to merge this to a branch for now so I can deploy it. |
Hi Shankari, sure but please make sure they works on your loocal machine. Unfortunately, I don't have time to review the PRs this week. |
While testing with this PR on my local laptop and a dataset with ~ 32 trips from one user in the past month, the process was still killed
On retrying, it worked
The map still didn't load because none of the entries had labels, so the |
@allenmichael099 how much data did you test this out on? |
switching to the month of Jan, and it was killed again.
I'm going to hold off on deploying this fix. |
Re-disabling the map view using the following patch for deployment since I still want to get the fix for #18 |
Still getting some kills. Looking at it a bit further - the process appears to be killed at this point:
on a successful load, we see
The related code is
The last
Why is merging with the summaries so data intensive? Each summary only has the first and last entry. |
While waiting for asiripanich#46 to be resolved
@allenmichael099 I have shared a mongodump of the data in the environment that generates this This is 100% reproducible. We should also talk a bit about thinking through various scenarios and adding unit tests |
- Fixed minimum trip date - moved trip_trend inside observeEvent so it reacts to a change in trips - commented out maps, locations, and trajectories - altered dates from input so the timestamps include all days that they should. - EX: When both dates are 5-06-21, the final query timestamp should be the start of 5-07-21
Potential unit tests:
|
@shankari I'm having trouble replicating the 'killed' error. Using the January 2021 data with 36 participants the server calls merged fine. |
ok, I will deploy this version and see if I run into the |
- made functions to query get/put/diary first and last server calls - used those in an altered summarise_server_calls - removed query_server_calls lines from mod_load_data - uncommented map, locations, and trajectories sections - added details to merge and timestamp messages
- when there are no user inputs, no user input columns will be added. - Wrote a message to notify when there are no user inputs
- Moved functions following the query out of query_cleaned_trips_by_timestamp Made tests for: - tidy_cleaned_trips_by_timestamp - get_query_size - summarise_trips_without_trips - summarise_sesrver_calls