-
Notifications
You must be signed in to change notification settings - Fork 2
tag_to_var_map arguments and tests #109
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
Changes from 17 commits
5298d45
829f502
ee7e965
dc96498
e0e049a
8c8f8e9
33cc834
dae7367
80652c9
1ee8006
26c4f5d
002e0e3
2edd1c0
32bc0cd
86b0a09
04b5b89
1bdfa6d
1cfaa2b
bc8a997
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -918,16 +918,32 @@ def process_unary_ops(self, data, tag_to_var_map={}): | |
| else: # must be a DataFrame | ||
| relevant_data = pd.Series([tag_obj.value] * len(data)) | ||
| elif isinstance(tag_obj, self.__class__): | ||
| relevant_data = tag_obj.calculate_values(data) | ||
| relevant_data = tag_obj.calculate_values(data, tag_to_var_map) | ||
| elif tag_to_var_map: | ||
| relevant_data = result[tag_to_var_map[tag_obj.id]] | ||
| else: | ||
| relevant_data = result[tag_obj.id] | ||
|
|
||
| relevant_data = unary_helper( # noqa: F405 | ||
| is_series = isinstance(relevant_data, pd.Series) | ||
| if is_series: # store info, then convert to np array | ||
| original_index = relevant_data.index | ||
| original_name = relevant_data.name | ||
| relevant_data = relevant_data.values | ||
|
|
||
| processed_relevant_data = unary_helper( # noqa: F405 | ||
| relevant_data, self.unary_operations[i] | ||
| ) | ||
|
|
||
| # Convert back if original was Series | ||
| if is_series: | ||
| relevant_data = pd.Series( | ||
| processed_relevant_data, | ||
| index=original_index, | ||
| name=original_name, | ||
| ) | ||
| else: | ||
| relevant_data = processed_relevant_data | ||
|
|
||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change was added because when using tag_to_var_map, relevant_data wouldn't already be a np array. Can think about if there are better ways to handle this
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems like a reasonable solution to me! |
||
| if tag_to_var_map: | ||
| result[tag_to_var_map[tag_obj.id]] = relevant_data | ||
| else: | ||
|
|
@@ -1015,7 +1031,7 @@ def process_binary_ops(self, data, tag_to_var_map={}): | |
| else: | ||
| relevant_data = data[tag_obj.id].copy() | ||
| elif isinstance(tag_obj, self.__class__): | ||
| relevant_data = tag_obj.calculate_values(data) | ||
| relevant_data = tag_obj.calculate_values(data, tag_to_var_map) | ||
| elif tag_to_var_map: | ||
| relevant_data = data[tag_to_var_map[tag_obj.id]].copy() | ||
| else: | ||
|
|
@@ -1077,7 +1093,7 @@ def process_binary_ops(self, data, tag_to_var_map={}): | |
| else: | ||
| relevant_data = data[tag_obj.id].copy() | ||
| elif isinstance(tag_obj, self.__class__): | ||
| relevant_data = tag_obj.calculate_values(data) | ||
| relevant_data = tag_obj.calculate_values(data, tag_to_var_map) | ||
| elif tag_to_var_map: | ||
| relevant_data = data[tag_to_var_map[tag_obj.id]].copy() | ||
| else: | ||
|
|
@@ -1155,7 +1171,7 @@ def process_custom_ops(self, data, tag_to_var_map={}): | |
| varname = tag_to_var_map[tag_obj.id] if tag_to_var_map else tag_obj.id | ||
| varnames.append(varname) | ||
| if isinstance(tag_obj, self.__class__): | ||
| data[varname] = tag_obj.calculate_values(data) | ||
| data[varname] = tag_obj.calculate_values(data, tag_to_var_map) | ||
| result = func_(*[data[varname] for varname in varnames]) | ||
| if isinstance(result, Series): | ||
| result.rename(self.id, inplace=True) | ||
|
|
@@ -1193,7 +1209,10 @@ def calculate_values(self, data, tag_to_var_map={}): | |
| data = self.process_binary_ops(data, tag_to_var_map=tag_to_var_map) | ||
| elif isinstance(data, (dict, DataFrame)): | ||
| # if no binary ops, get appropriate column from unary ops and rename | ||
| data = data[self.tags[0].id].rename(self.id) | ||
| if isinstance(data, dict): | ||
| data = pd.Series(data[self.tags[0].id], name=self.id) | ||
| else: | ||
| data = data[self.tags[0].id].rename(self.id) | ||
| elif isinstance(data, ndarray): | ||
| # flatten array since binary operations do that automatically | ||
| data = data[:, 0] | ||
|
|
@@ -1202,7 +1221,10 @@ def calculate_values(self, data, tag_to_var_map={}): | |
| data = self.process_custom_ops(data, tag_to_var_map=tag_to_var_map) | ||
| elif isinstance(data, (dict, DataFrame)): | ||
| # if custom_operations is empty, get appropriate column and rename | ||
| data = data[self.tags[0].id].rename(self.id) | ||
| if isinstance(data, dict): | ||
| data = pd.Series(data[self.tags[0].id], name=self.id) | ||
| else: | ||
| data = data[self.tags[0].id].rename(self.id) | ||
| elif isinstance(data, ndarray): | ||
| # flatten array since operations do that automatically | ||
| data = data[:, 0] | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| GasToBoiler_NaturalGas_Flow,GasToCogen_NaturalGas_Flow,ConditionerToCogen_Biogas_Flow,CogenElecToFacility_Electricity_Flow,Constant1 | ||
| 0,1264.064032,6361.096418,919,1 | ||
| 0,1304.840291,6361.096418,863,1 | ||
| 0,1304.840291,6034.886346,882,1 | ||
| 0,1264.064032,5994.110087,945,1 | ||
| 0,1223.287773,6075.662605,914,1 | ||
| 0,1223.287773,6605.753973,912,1 | ||
| 0,1264.064032,5831.00505,867,1 | ||
| 0,1345.61655,6157.215123,926,1 | ||
| 0,1223.287773,5790.228791,890,1 | ||
| 0,1264.064032,6483.425196,911,1 | ||
| 0,1264.064032,6687.306491,901,1 | ||
| 0,1223.287773,6157.215123,926,1 | ||
| 0,1223.287773,6361.096418,916,1 | ||
| 0,1304.840291,6809.635268,925,1 | ||
| 0,1386.392809,6850.411528,890,1 | ||
| 0,1223.287773,6809.635268,935,1 | ||
| 0,1264.064032,6687.306491,913,1 | ||
| 0,1304.840291,6646.530232,905,1 | ||
| 0,1264.064032,6646.530232,927,1 | ||
| 0,1264.064032,6687.306491,921,1 | ||
| 0,1223.287773,6972.740305,903,1 | ||
| 0,1223.287773,6564.977714,886,1 | ||
| 0,1304.840291,5953.333828,911,1 | ||
| 0,1264.064032,6157.215123,896,1 | ||
| 0,1304.840291,5667.900014,879,1 | ||
| 0,1264.064032,5260.137423,915,1 | ||
| 0,1304.840291,5423.242459,915,1 | ||
| 0,1304.840291,5341.689941,882,1 | ||
| 0,1304.840291,5137.808646,930,1 | ||
| 0,1264.064032,4811.598573,889,1 | ||
| 0,1264.064032,5056.256127,890,1 | ||
| 0,1264.064032,5056.256127,897,1 | ||
| 17.488486,1182.511514,5097.032387,867,1 | ||
| 17.488486,1182.511514,5504.794977,878,1 | ||
| 302.9223,897.0777,4730.046055,939,1 | ||
| 343.6985591,856.3014409,4566.941018,853,1 | ||
| 1200,0,4730.046055,862,1 | ||
| 1200,0,5056.256127,911,1 | ||
| 1200,0,5219.361164,816,1 | ||
| 1200,0,5545.571237,879,1 | ||
| 1200,0,5341.689941,905,1 | ||
| 1200,0,5545.571237,961,1 | ||
| 1200,0,5178.584905,919,1 | ||
| 1200,0,5871.781309,903,1 | ||
| 1200,0,5953.333828,944,1 | ||
| 1200,0,5871.781309,909,1 | ||
| 1200,0,5790.228791,927,1 | ||
| 1200,0,5464.018718,910,1 | ||
| 1200,0,5545.571237,903,1 | ||
| 1200,0,5667.900014,917,1 | ||
| 1200,0,5382.4662,886,1 | ||
| 1200,0,5871.781309,907,1 | ||
| 1200,0,5708.676273,897,1 | ||
| 1200,0,5667.900014,966,1 | ||
| 1200,0,5627.123755,893,1 | ||
| 1200,0,5300.913682,893,1 | ||
| 1200,0,5586.347496,953,1 | ||
| 1200,0,5056.256127,924,1 | ||
| 1200,0,5300.913682,908,1 | ||
| 1200,0,5015.479868,935,1 | ||
| 1200,0,5464.018718,976,1 | ||
| 1200,0,5464.018718,923,1 | ||
| 1200,0,5464.018718,895,1 | ||
| 1200,0,5464.018718,910,1 | ||
| 1200,0,5341.689941,953,1 | ||
| 1200,0,4770.822314,595,1 | ||
| 1200,0,5178.584905,946,1 | ||
| 1200,0,5341.689941,926,1 | ||
| 1200,0,5831.00505,959,1 | ||
| 1200,0,5219.361164,916,1 | ||
| 1200,0,5382.4662,887,1 | ||
| 1200,0,5137.808646,840,1 | ||
| 1200,0,5382.4662,863,1 | ||
| 1200,0,4485.3885,881,1 | ||
| 1200,0,5341.689941,863,1 | ||
| 1200,0,5097.032387,799,1 | ||
| 1200,0,4770.822314,819,1 | ||
| 0,1304.840291,5097.032387,840,1 | ||
| 0,1264.064032,4811.598573,877,1 | ||
| 0,1304.840291,4893.151091,918,1 | ||
| 0,1264.064032,5260.137423,939,1 | ||
| 0,1223.287773,5260.137423,956,1 | ||
| 0,1264.064032,4240.730946,933,1 | ||
| 0,1264.064032,4974.703609,889,1 | ||
| 17.488486,1182.511514,5382.4662,921,1 | ||
| 0,1223.287773,4933.92735,938,1 | ||
| 17.488486,1182.511514,4974.703609,896,1 | ||
| 0,1223.287773,4566.941018,852,1 | ||
| 0,1223.287773,4526.164759,925,1 | ||
| 0,1264.064032,4811.598573,914,1 | ||
| 0,1223.287773,4770.822314,892,1 | ||
| 0,1223.287773,4363.059723,903,1 | ||
| 0,1223.287773,4811.598573,909,1 | ||
| 0,1223.287773,5178.584905,950,1 | ||
| 0,1223.287773,4648.493537,926,1 | ||
| 17.488486,1182.511514,5178.584905,919,1 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| { | ||
| "ElectricityGeneration_LShift1": "ElectricityGeneration_LShift1", | ||
| "Digester3GasFlow": "ConditionerToCogen_Biogas_Flow", | ||
| "Digester2GasFlow": "ConditionerToCogen_Biogas_Flow", | ||
| "BoilerGasPurchases": "GasToBoiler_NaturalGas_Flow", | ||
| "TotalizedFlaredGas": "ConditionerToFlare_Biogas_Flow", | ||
| "NoGasPurchases": "NoGasPurchases", | ||
| "GrossGasProduction": "GrossGasProduction", | ||
| "ElectricityProductionByGasVolume": "ElectricityProductionByGasVolume", | ||
| "ElectricityGeneration_RShift2_List": "ElectricityGeneration_RShift2_List", | ||
| "Digester1GasFlow": "ConditionerToCogen_Biogas_Flow", | ||
| "CogenGasPurchases": "GasToCogen_NaturalGas_Flow", | ||
| "CombinedDigesterGasFlow": "ConditionerToCogen_Biogas_Flow", | ||
| "ElectricityGeneration": "CogenElecToFacility_Electricity_Flow", | ||
| "TankLevel": "FOGTank_FatOilGrease_Level", | ||
| "InfluentFlow": "SewerIntake_UntreatedSewage_Flow", | ||
| "NoGasPurchasesList": "NoGasPurchasesList", | ||
| "ElectricityGeneration_LShift1_List": "ElectricityGeneration_LShift1_List", | ||
| "ElectricityGeneration_RShift2": "ElectricityGeneration_RShift2", | ||
| "TankVolume": "FOGTank_FatOilGrease_Volume", | ||
| "GrossGasProductionList": "GrossGasProductionList", | ||
| "ElectricityGenDelta": "ElectricityGenDelta" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| TestEmptyCustom | ||
| 919.0 | ||
| 863 | ||
| 882 | ||
| 945 | ||
| 914 | ||
| 912 | ||
| 867 | ||
| 926 | ||
| 890 | ||
| 911 | ||
| 901 | ||
| 926 | ||
| 916 | ||
| 925 | ||
| 890 | ||
| 935 | ||
| 913 | ||
| 905 | ||
| 927 | ||
| 921 | ||
| 903 | ||
| 886 | ||
| 911 | ||
| 896 | ||
| 879 | ||
| 915 | ||
| 915 | ||
| 882 | ||
| 930 | ||
| 889 | ||
| 890 | ||
| 897 | ||
| 867 | ||
| 878 | ||
| 939 | ||
| 853 | ||
| 862 | ||
| 911 | ||
| 816 | ||
| 879 | ||
| 905 | ||
| 961 | ||
| 919 | ||
| 903 | ||
| 944 | ||
| 909 | ||
| 927 | ||
| 910 | ||
| 903 | ||
| 917 | ||
| 886 | ||
| 907 | ||
| 897 | ||
| 966 | ||
| 893 | ||
| 893 | ||
| 953 | ||
| 924 | ||
| 908 | ||
| 935 | ||
| 976 | ||
| 923 | ||
| 895 | ||
| 910 | ||
| 953 | ||
| 595 | ||
| 946 | ||
| 926 | ||
| 959 | ||
| 916 | ||
| 887 | ||
| 840 | ||
| 863 | ||
| 881 | ||
| 863 | ||
| 799 | ||
| 819 | ||
| 840 | ||
| 877 | ||
| 918 | ||
| 939 | ||
| 956 | ||
| 933 | ||
| 889 | ||
| 921 | ||
| 938 | ||
| 896 | ||
| 852 | ||
| 925 | ||
| 914 | ||
| 892 | ||
| 903 | ||
| 909 | ||
| 950 | ||
| 926 | ||
| 919 |
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.
Moved this TypeError to the end to simplify when this is raised and help with codecov... alternatively can just add separate tests for each operation type
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.
Does this work? I would think that there would be an
UnboundLocalErrorbecause the variableresultmay be referenced without being defined.Besides that bug, I'm all for streamlining the logic
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.
Good catch!! That did raise an UnboundLocalError.
The test in test_calculate_values was passing because a TypeError was being raised earlier, from process_unary_ops in tag.py. So unary_helper was never even called
I removed that type checking entirely instead, since unary_helper shouldn't ever be called outside process_unary_ops... is that ok or do you prefer to keep it as it was originally just in case?