Improve robustness of check_df and code refinements#3
Conversation
… or Flowable columns.
tur-ium
changed the title
bl-young
left a comment
bl-young
left a comment
There was a problem hiding this comment.
Thanks for your help here! This looks good, see my few requests/comments.
| ] | ||
|
|
||
| [tool.uv] | ||
| exclude-newer = "1 week" |
There was a problem hiding this comment.
I don't believe this is a valid entry
| !import_emission_factors/output/*import_factors_exiobase_2019* | ||
| !import_emission_factors/output/aggregate* | ||
| !import_emission_factors/output/import_shares_2019* | ||
| # Standard python gitignore from https://github.com/github/gitignore/ |
There was a problem hiding this comment.
I am not opposed to moving to the more standard .gitignore but I'd like to keep the existing ignores in there as well for now
| "fedelemflowlist", | ||
| "numpy", | ||
| "openpyxl", | ||
| "pandas", |
There was a problem hiding this comment.
| "pandas", | |
| "pandas[excel]", |
I get an ImportError as pandas requires openpyxl >= 3.1.5. I think setting this optional install should resolve it.
| "numpy", | ||
| "openpyxl", | ||
| "pandas", | ||
| "pymrio", |
There was a problem hiding this comment.
| "pymrio", | |
| "pymrio", | |
| "python-dotenv", |
Let's go ahead and add python-dotenv as well.
| ## summary sectors. | ||
| multiplier_df = df_prepare(multiplier_df, year) | ||
| check = (multiplier_df | ||
| .query('Flow == @multiplier_df["Flow"][0]') |
There was a problem hiding this comment.
Was this chunk causing you problems? I think it was an oversight. We should probably always use Flowable instead of Flow which should work for all instances (there's no need to have both options in here).
2 participants
In generate_import_factors.py Improve robustness of check_df to input data format (support Flow or Flowable as column names)
Quality-of-life and code security improvements