Closed iantei closed 5 months ago
Apparently, there is some issue with the above implementation. The above implementation relies on the data to fetch the labels for MODE
, REPLACED_MODE
& PURPOSE
. So, any changes in the labelling of the data further down would change the color mapping.
Therefore, a new function mapping_color_labels(dynamic_labels, dic_pur, dic_re)
has been created in scaffolding.py
which uses the stable list of labels derived from auxillary_files/mode_labels.csv
and purpose_labels.csv
for default mapping and uses dynamic labels
retrieved from the config file for individual program/study for the dynamic mapping.
Test Scenario:
Loaded Dataset `openpath_prod_cortezebikes`, `openpath_prod_usaid_laos_ev`:
Test have been executed for the following:
A. usaid-laos
- study with dynamic mapping
B. cortezebikes
- program with default mapping and e-bike
as mode_of_interest
.
Testing with usaid-laos:
For `usaid-laos-ev`: ``` - DB_HOST=mongodb://db/openpath_prod_usaid_laos_ev - WEB_SERVER_HOST=0.0.0.0 - CRON_MODE=TRUE - STUDY_CONFIG=usaid-laos-ev ``` For `cortezebikes`: ``` - DB_HOST=mongodb://db/openpath_prod_cortezebikes - WEB_SERVER_HOST=0.0.0.0 - CRON_MODE=TRUE - STUDY_CONFIG=cortezebikes ```
For `usaid-laos`:
```
(emission) root@4135283cdfaf:/usr/src/app/saved-notebooks# PYTHONPATH=.. python bin/update_mappings.py mapping_dictionaries.ipynb
(emission) root@4135283cdfaf:/usr/src/app/saved-notebooks# PYTHONPATH=.. python bin/generate_plots.py generic_metrics.ipynb default
/usr/src/app/saved-notebooks/bin/generate_plots.py:30: SyntaxWarning: "is not" with a literal. Did you mean "!="?
if r.status_code is not 200:
About to download config from https://raw.githubusercontent.com/e-mission/nrel-openpath-deploy-configs/main/configs/usaid-laos-ev.nrel-op.json
Successfully downloaded config with version 1 for USAID-NREL Support for Electric Vehicle Readiness and data collection URL https://USAID-laos-EV-openpath.nrel.gov/api/
Dynamic labels download was successful for nrel-openpath-deploy-configs: usaid-laos-ev
Running at 2024-02-10T16:44:10.010472+00:00 with args Namespace(plot_notebook='generic_metrics.ipynb', program='default', date=None) for range (
For `usaid-ebikes`:
```
(emission) root@33d99e64b921:/usr/src/app/saved-notebooks# PYTHONPATH=.. python bin/generate_plots.py generic_metrics.ipynb default
/usr/src/app/saved-notebooks/bin/generate_plots.py:30: SyntaxWarning: "is not" with a literal. Did you mean "!="?
if r.status_code is not 200:
About to download config from https://raw.githubusercontent.com/e-mission/nrel-openpath-deploy-configs/main/configs/usaid-laos-ev.nrel-op.json
Successfully downloaded config with version 1 for USAID-NREL Support for Electric Vehicle Readiness and data collection URL https://USAID-laos-EV-openpath.nrel.gov/api/
Dynamic labels download was successful for nrel-openpath-deploy-configs: usaid-laos-ev
Running at 2024-02-10T17:00:13.941489+00:00 with args Namespace(plot_notebook='generic_metrics.ipynb', program='default', date=None) for range (
Comparison of results:
For usaid-laos-ev |
Previous | Current |
---|---|---|
For cortezebikes |
Previous | Current |
---|---|---|
Conclusion:
PURPOSE
, and MODE
& REPLACED_MODE
representation.https://github.com/e-mission/em-public-dashboard/pull/113#issuecomment-1936854567 The commit history is not correct
Actually, I take that back. The commit history is clean, just more expansive than I had expected.
@iantei this is a very clean PR, but it only addressed the charts displaying labeled data. Where are the changes for the sensed metrics? How are you making sure that they are internally consistent - e.g. between trip count and trip miles?
I know that we currently display the sensed charts by default, so I am also not sure why they didn't show up when you tried testing. All the charts displayed here only showcase labeled data.
@iantei this is a very clean PR, but it only addressed the charts displaying labeled data. Where are the changes for the sensed metrics?
labeled
chart, I didn't make changes for it in this PR. I will update the required changes for Stacked Bar Charts, when I integrate these color scheme changes.I know that we currently display the sensed charts by default, so I am also not sure why they didn't show up when you tried testing. All the charts displayed here only showcase labeled data.
I am not modifying any changes related with sensed charts, or changing the default loading of charts in index.html
. Therefore, I limited my comparison scope just to the pie charts which have been impacted.
How are you making sure that they are internally consistent - e.g. between trip count and trip miles?
value
parameter, which would change on the basis of different filters. Now, I have generated two distinct dictionary i.e. one for purpose
related and other combination of both replaced mode
and mode
related label. This way, the labels for trip count
and trip miles
which are both based on mode
will have same color mapping dictionary.
Moreover, since Replaced mode
based charts and Mode
based charts have identical labels, these labels are merged to create a combined dictionary, therefore we will have uniformity across these chart label's color palette too.@iantei what is the question here?
I didn't have any code changes from the last review comment, which needed to be reviewed. But still wanted your response on my comments, therefore moved it to the question column instead. I just wanted a follow up on it regarding whether these responses are satisfactory or not.
We already have a defined mapping for sensed charts. And their labels are completely different from labeled chart, I didn't make changes for it in this PR. I will update the required changes for Stacked Bar Charts, when I integrate these color scheme changes.
Here's the note/comment that would have helped clarify this.
Note that in pie_chart_sensed_mode
, the color map is generated from a static array of all_labels
all_labels= ['IN_VEHICLE',
...
colours = dict(zip(all_labels, plt.cm.tab10.colors[:len(all_labels)]))
Instead of the current pie charts, where the colormap is defined from the values that are passed in to the function
def pie_chart_mode(plot_title,labels,values,file_name):
...
colours = dict(zip(labels, plt.cm.tab20.colors[:len(labels)]))
@iantei this is merged. please incorporate the changes into the bar chart PR so I can merge that and unblock @Abby-Wheelis
Test Scenario:
Changes made in
docker-compose.yml
for the above testing:Executed scripts to load charts from
Generic Metrics
&Mode Specific Metrics
notebooks:Results:
Conclusion: Now, with this change. Different labels across all charts in the public dashboard will have same color. For example, two charts showing
purpose
above coming form different notebook related trip metrics have same color coding. Note: We will have different color mapping for mode, purpose, and replaced mode charts since we have three distinct list of these labels and mapping.