Skip to content
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

Merge helpers #1029

Closed
wants to merge 38 commits into from
Closed

Conversation

finozzifa
Copy link
Contributor

@finozzifa finozzifa commented May 22, 2024

Closes # (if applicable).

Together with the merge of the methods from the helpers.py script of pypsa-earth-sec into the _helpers.py script of pypsa-earth, the pull request proposes also a possible solution for pypsa-meets-earth/pypsa-earth-sec#310.

Changes proposed in this Pull Request

This is the first pull request I propose for the merge of pypsa-earth-sec into pypsa-earth. The changes are listed below:

Changes in _helpers.py

  • two_digits_2_name_country : I add a new function argument (called name_string) to specify whether we wish to return the full name of the country or its full name. For example when name_string=name_short CD -> DR Congo, whereas when name_string=name_official CD -> Democratic Republic of the Congo
  • prepare_costs: I add this method from the helpers.py scripts of pypsa-earth-sec
  • create_network_topology: I add this method from the helpers.py script of pypsa-earth-sec
  • cycling_shift: I add this method from the helpers.py script of pypsa-earth-sec
  • download_gadm: I add this method from the helpers.py script of pypsa-earth-sec
  • get_gadm_layer: I add this method from the helpers.py script of pypsa-earth-sec
  • locate_bus: I add this method from the helpers.py script of pypsa-earth-sec
  • safe_divide: I add this method from the helpers.py script of pypsa-earth-sec

Proposed resolution of issue pypsa-meets-earth/pypsa-earth-sec#310

I created the issue pypsa-meets-earth/pypsa-earth-sec#310 in pypsa-earth-sec. In a nutshell, one of the inputs used in pypsa-earth-sec for rule build_base_energy_totals contains a few typos (e.g. "Hrad coal") and many duplicated entries (e.g. "Coke-oven coke", "Coke oven coke" and "Coke Oven Coke"). Such typos and duplicated entries have been propagated in the method get_conv_factors and in the method aggregate_fuels. Moreover, aggregate_fuels contains two mutually exclusive definitions for the list coal_fuels. Finally, the method aggregate_fuels does not use the function argument sector.

The above-mentioned input is composed of several text files, which are read into a pandas dataframe. The processing happens in scripts/build_base_energy_totals.py in pypsa-earth-sec. The entries "Hrad coal", "Coke-oven coke", "Coke oven coke" and "Coke Oven Coke" etc. populate the column "Commodity" of the pandas dataframe.

My proposed solution is to leave the input files untouched and to modify the pandas dataframe, replacing the typos and lower-casing all entries of column "Commodity". I implement the solution with the following methods:

  • modify_commodity: I create the method to replace the typos and lower-case the entries of the "Commodity" column
  • get_conv_factors: I add this method from the helpers.py script of pypsa-earth-sec. I lower case all keys of the dictionary and correct the typos.
  • aggregate_fuels: I add this method from the helpers.py script of pypsa-earth-sec. I lower case all entries of the lists, I unify the mutually exclusive definitions of list coal_fuels into one. I modify the method such that it returns the defined lists when sector==industry. This is because get_conv_factors and aggregate_fuels are used in conjunction. Hence it makes sense that they both have the same behavior with respect to the value of sector.

Unit tests for the _helpers.py methods

I have added dedicated unit tests for some of the methods present in the _helpers.py script. In particular I test that the modify_commodity method yields the requested changes to the "Commodity" column of the pandas dataframe.

Further notes

  • create_dummy_data(n, sector, carriers): I did not copy this method from pypsa-earth-sec/scripts/helpers.py because it is not used in pypsa-earth-sec
  • get_country(target, **keys): this method from pypsa-earth-sec/scripts/helpers.py is not needed

Checklist

  • I consent to the release of this PR's code under the AGPLv3 license and non-code contributions under CC0-1.0 and CC-BY-4.0.
  • I tested my contribution locally and it seems to work fine.
  • Code and workflow changes are sufficiently documented.
  • Newly introduced dependencies are added to envs/environment.yaml and doc/requirements.txt.
  • Changes in configuration options are added in all of config.default.yaml and config.tutorial.yaml.
  • Add a test config or line additions to test/ (note tests are changing the config.tutorial.yaml)
  • Changes in configuration options are also documented in doc/configtables/*.csv and line references are adjusted in doc/configuration.rst and doc/tutorial.rst.
  • A note for the release notes doc/release_notes.rst is amended in the format of previous release notes, including reference to the requested PR.

finozzifa and others added 30 commits May 15, 2024 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant