Oil refineries ng processing #51
Conversation
Creation of the ng_processing_proxy and oil_refineries_proxy
Added uniform distribution of emissions across state for states missing proxy data.
Changed Header to be consistent with other proxy headers. Removed imports that were unused (please verify with author that these were not used): - os - pyarrow.parquet - V3_DATA_PATH Added function docstring with Args Questions: - Are lines 388 and 394 redundant? It may not impact the script performance, but I wanted to check. - Lines 441, 442. Should line 441 be commented out as well if assert is commented out on line 442, as sums may not need to be calculated if it is not being asserted. Ignored lines >88 char due to object name lengths and process complexity. Added some documentation context, but overall code was commented well.
Changed Header to make consistent with other proxy scripts. Removed parquet import, unused in script. Added pytask function docstring. Minor edits to bring some lines <88 char. Added some documentation in larger chunks. Overall well documented.
|
@haclohman I committed updates for task_ng_processing_proxy and task_oi_refineries_proxy. Overall, scripts were well documented. I added notes to commit edits. Primary edits were to header and chunk documentation to make similar to other proxy scripts. Let me know if you approve the edits or want to follow up. Then we can ping Nick to review/merge. |
There was a problem hiding this comment.
Answers to your questions:
-
Are lines 388 and 394 redundant? It may not impact the script performance, but I wanted to check. They are redundant. I forgot to delete one when I was reordering that section. Feel free to delete one!
-
Lines 441, 442. Should line 441 be commented out as well if assert is commented out on line 442, as sums may not need to be calculated if it is not being asserted. We could comment it out. I had it commented out because I was getting the error but troubleshooted it.
|
I think this PR can be merged into main now. I reviewed both code updates and they look good to me. |
Removed redundant code, per conversation with author.
|
I removed the redundant code at line 394 for task_ng_processing_proxy. I left the other questioned code in because it isn't impacting functioning. @nkruskamp Should be good to merge. |
|
@nkruskamp I'm going to update the processing proxy to correct the state code-year pair mismatch between emi and proxy before we merge. |
Updated ng_processing proxy file to correct mismatch between state code-year pairs.
|
@nkruskamp this branch should be ready to merge with the correction to the processing proxy now complete. |
ng_processing_proxy and oil_refineries_proxy