-
Notifications
You must be signed in to change notification settings - Fork 23
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 base mode and footprint to the labeled trips dataframe #152
base: main
Are you sure you want to change the base?
Conversation
I can't see anything obviously wrong. Let me catch up on a few emails and then try this as a separate snippet... |
I think the issue is the way I iterate over them, not the way that I insert information. I tried iterating over them and not modifying them, and had the same issue |
this works for me
I wonder if the issue is that you are trying to iterate over the iterator twice. Note that |
This was exactly the what I was missing. I converted the iterator to a list, like in your snippet, and things work now. I have successfully added |
the iterator is consumed on first iteration, must use a list to update items instead
The data is coming together! There is definitely room for polishing especially in regard to what further unification could take place with |
having separate columns makes accessing the data easier, no longer need to break a tuple
remove most of the old footprint calculations unpack the footprints into energy and emissions calculate difference between mode and replaced mode
data function is now async because of waiting on returns from e-mission-common
using older names allows for old code to run
only calculating the footprint when needed in a given notebook saves time
updated the version in the build script and updated the calls in scaffolding.py to use the new call pattern some slight data formatting required
I still need to make this compatible with dynamic labels! I might need help testing that piece unless I can get my database to work better than it currently is... |
@Abby-Wheelis I think I have merged all the dependencies (base mode changes and inferred labels). You should be unblocked to finish up this change. |
Not broken after merge! On to trying to rip out the dictionaries so we can pass the labels the same way for all deployments - default or custom per-deployment |
Now relying on the default labels from emcommon
Ok, I've removed the csvs now, I think we could further reduce duplication by moving the choice of labels into |
@iantei can you test this branch on your configuration? My Docker is still uncooperative and we should get this into review as soon as we can. There is still more cleanup to be done, but we can host that in a separate PR. We need to make sure that it works for 1) default labels 2) deployment-custom labels 3) surveys (no footprint will be shown, but want to make sure nothing broke) |
…ary_files dir anymore.
…lumns before calling unpack_energy_emissions().
…npack_energy_emissions look up for mode_confirm_footprint over mode_confirm.
… for program, not study.
Testing Stacked Bar Charts for Cortez ebikes with the merged and current changes after ea3cd89: Comparison between Cortez ebikes current and merged
Looks identical. Note: The color variants from base mode appears to be picked up differently each time the charts are being generated. Might be something about dedupe_colors() which needs to be explored. |
Testing Scenario: Dataset used: Following notebooks are executed:
Detailed execution of above notebooks using `generate_plots.py` script:
Results:
All the charts generation ran smoothly. |
Test Scenario: Dataset used: Executed the following notebooks:
Changes in the config file:
In
Details of execution of the above notebooks:
Could not run the below notebooks:
because of the expected error:
Results:
All charts generated properly. Note: There is issue with running generic_metrics notebook with dfc-fermata data, it looks up for |
…ated data for generic_metrics noteboook.
Incorporated - Do not execute generic_metrics notebook for survey info. Details
|
I have tested with all three configurations (1), (2), & (3). I have not tallied the data representation in the charts against older version. |
viz_scripts/generic_metrics.ipynb
Outdated
"metadata": {}, | ||
"outputs": [], | ||
"source": [ | ||
"# Do not run this notebook if it has survey_info; nbclient will run up through this cell\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't filter on if survey info exists -
Some deployments have trip-level surveys: dfc-fermata
And others have records of the onboarding survey: cortezebikes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in 468d40e by filtering as
if not survey_info.get('trip-labels', None) == 'MULTILABEL':
\\ Do not execute the generic_metrics notebook.
…TILABEL for generic_metrics notebook
Testing Scenario: Dataset: Execution of generate_plots.py for generic_metrics notebook Execution details alongside the git diff for changing `STUDY_CONFIG` and `DB_HOST` in docker-compose.yml :
When running the generate_plots.py for generic_metrics for
This addresses this comment #148 (comment) too |
See initial discussion in #146