Adding by_charge_key argument to calculate_itemized_cost, and some bug fixes#51
Conversation
…to also return itemized costs by charge key if requested. (for use with flows-mpc) Adding missing pytz requirement for tests adding copy of charge in costs.py during unit conversion to avoid pandas issue Fixing read-only issue in hours definition in get_charge_dict and create_charge_array. Also in test_create_charge_array. Issue is that with pandas 3.0.0, datetime.dt.hours.values is read-only and can't have minutes added Reformatted with black Tests passing with python 3.11 and pandas 3.0
Integrating total_cost into calculate_itemized_cost loop since dict has a different structure with by_charge_key True
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #51 +/- ##
==========================================
+ Coverage 93.40% 94.38% +0.97%
==========================================
Files 10 10
Lines 1593 1603 +10
==========================================
+ Hits 1488 1513 +25
+ Misses 105 90 -15 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…s data has no_day_var=False and rollover=True across months Adding more specific charge key checks in test_costs updates for by_charge_key
fletchapin
left a comment
fletchapin
left a comment
There was a problem hiding this comment.
Just one question about copying the charges
| # if not specified, assume the old format (imperial units) | ||
| if utility == GAS: | ||
| # convert from therms to default units of cubic meters | ||
| charge = charge.copy() |
There was a problem hiding this comment.
What is the goal of copying this dictionary? Was there a bug that you found?
There was a problem hiding this comment.
This came up when running with pandas 3.0 from flows-mpc, which had the older billing format in tests.
It was raising a ChainedAssignmentError: https://pandas.pydata.org/docs/user_guide/copy_on_write.html#chained-assignment
Instead of making a copy, we could possibly use .loc (like the linked example) if modifying limit_charges directly instead of charge, but that would probably be more complicated.
I ended up changing the flows-mpc billing csvs to the new format so it's not actually hitting this code anymore. Think it's worth adding a test case for this "except" section?
3 participants
Main update: adding by_charge_key argument to calculate_itemized_cost in costs.py, to also return itemized costs by charge key if requested. (for use with flows-mpc)
Adding test for by_charge_key = True
Other bug fixes:n
Reformatted with black
Tests passing with python 3.11 and pandas 3.0
Pull request recommendations:
Thanks for contributing!