🔥 🧹 Bugfix and cleanup for "Adding Surveys" #135
Conversation
moved survey_list initialization out of the if block so it would be defined (just empty) on non survey deployments and prevent the breaking error on the regular deployments
push the data loading process into scaffolding.py (where it belongs) and update process to match convention we load all participant trips, then filter to the trips with input, and return both filtered and unfiltered frames filtered is used for visualization, unfiltered is required for the quality text stats
as a part of centralizing the data loading, move the formatting of data into the same cell
I had created two functions, one in the notebook, one in scaffolding, I was using the one in the notebook, but thought I was using the one in scaffolding This created a catastrophic error when trying to fetch the colors for the charts - fixed now!
|
After some chaos with prepping the dashboard for demo this afternoon, the error was the result of a mistake that I made: There was a function to create the color mapping based on the options, but I had defined it twice - once in scaffolding and once in the notebook - when there was an error with I can now run both Will now test this cleanup so we can get the fix out for |
|
Testing done: ran notebooks on the command line for
The metrics also work: Additionally, on staging we see (with a failure to load charts): That error is not present in the console (and charts show, just blank since I have not run them) when I input the Bugs are resolved, and updates to data loading have been made |
|
I fixed the data issue and pulled new data. This is now generating everything except except which I think is expected since they were not even shown
Since the data has been fixed, I am going to also re-enable the |
|
Fixed metrics to use the Most trips are still UNKNOWN since they were taken before the BLE integration was complete. I wonder if we should remove some of them to make this data more meaningful.
|
- Comment in the previous code for using the `ble_sensed_mode` - Fixed color issues; we cannot use `colors_sensed` any more since the `ble_sensed_mode` will not be one of the 5 hardcoded values. So we find the set of unique values using a `groupby+agg` and then create a fake dictionary of the unique keys mapped to themselves to pass in. Note that the returned value will be `colors_mode` while we need to use `colors_sensed` in the graphs to handle the non-fleet case. So for the fleet case, we set `colors_sensed` to `color_mode` Now that I type that out, I see that the better fix would be to have a `colors_sel` that is set appropriately for fleet/non-fleet and then pass in `colors_sel` instead of `colors_mode`. But that would be a bigger change, so will leave it for @Abby-Wheelis to handle in the next round of changes Testing done: e-mission#135 (comment)
| "if len(survey_trips) > 0:\n", | ||
| " df_responses = create_dataframe(survey_trips)\n", | ||
| "else:\n", | ||
| " df_responses = survey_trips.copy()" |
There was a problem hiding this comment.
nit: I would suggest a more descriptive name than create_dataframe (create what kind of dataframe)
and it looks like the else is just setting up df_responses as an empty dataframe (if the length is zero, it is empty). I would use pd.DataFrame() to highlight that or even pd.DataFrame(columns=survey_trips.columns) just to make it more clear what is going on.
|
Another cleanup that needs to happen is to rework the legends. The limit of 15 characters may be too aggressive, particularly if there is a y label, or maybe we can put it on top where there is no label. And in this case, because there were actually ~ 10 entries, some of them got cutoff. We also needed to use nCols. I think we will have to think through the various use cases and create the legend accordingly. From the current staging... |
|
I also think for multi-label charts, we can change the alt-html to have the tables side by side. Right now, they are one below the other which leaves a lot of empty space. |
|
Also, we have "testers and participants" in some locations, but hardcoded "users" in others. Need to unify.
|
When I checked staging this morning, I realized a change I made to display surveys broke non-survey deployments, fixed this in ensure survey_list is defined
Also addressing comments from review of #124, including refactor of data loading to better match the established convention