transportenergy / database

Tools for accessing and maintaining the iTEM model & historical databases
https://transportenergy.rtfd.io
GNU General Public License v3.0
24 stars 8 forks source link

Add scripts for T004–T008 #18

Closed hlinero closed 4 years ago

hlinero commented 4 years ago

This pull request provides the following functionalities:

  1. Created a better folder structure to organize the util files.
  2. Expanded the functionality of the util mode that can be reused by other scripts.
  3. ~Added json files corresponding to the ISO country codes and ITEM regions.~ pdated the data submodule to:
    • make use of common region definitions, and
    • store input data (temporary).
  4. Completed the T004 through T008 scripts.
  5. Ensure scripts can be run on CI.
codecov[bot] commented 4 years ago

Codecov Report

Merging #18 into master will decrease coverage by 7.49%. The diff coverage is 86.88%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master     #18     +/-   ##
========================================
- Coverage   56.79%   49.3%   -7.5%     
========================================
  Files          11      29     +18     
  Lines         905    1363    +458     
========================================
+ Hits          514     672    +158     
- Misses        391     691    +300
Impacted Files Coverage Δ
item/tests/test_cli.py 0% <ø> (ø)
item/tests/test_historical.py 0% <0%> (ø)
item/historical/__init__.py 20.51% <100%> (+1.03%) :arrow_up:
item/historical/cli.py 79.31% <50%> (-15.69%) :arrow_down:
item/historical/util.py 95% <95%> (ø)
item/historical/scripts/util/managers/dataframe.py 96.05% <96.05%> (ø)
...m/historical/scripts/util/managers/country_code.py 98.38% <98.38%> (ø)
item/tests/test_dimensions.py 0% <0%> (ø)
item/model/get.py 0% <0%> (ø)
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 309c26d...a4e198a. Read the comment docs.

khaeru commented 4 years ago

Added json files corresponding to the ISO country codes and ITEM regions

Hi @hlinero (also @soniayeh FYI)—this is duplicative, in two ways:

  1. pycountry exists to handle ISO 2166 codes. It is much more robust for us to rely on this long-existing, regularly-updated, and well-tested library than to keep a copy of the codes in a file. So, please remove iso_codes.json as well as any custom code for handling it, and make use of this library.
  2. The transportenergy/metadata repo exists to contain metadata. The metadata repo is included as a submodule in this repo at item. It has several files related to iTEM regions, including regional mappings for particular models, and for the iTEM2 and iTEM3 model databases. Please:
    • Remove item_regions.json.
    • Look at the existing Python code in item.model that loads these files and returns data structures. If the code is not suitable for your purposes in the PR, propose some modifications via code: e.g. expand the functionality of the existing methods (e.g. with new arguments), relocate them (e.g. from item.model to a common module like item.utils), or propose new wrappers. In any case, do not duplicate it.

If our code is to be readable, maintainable, well-tested, and ultimately scientifically valid, it is essential that we avoid situations where we use separate copies of the same metadata via two different code paths.

In both cases, please ask if you have any questions; then let me know when you've made these changes and I will look at the remainder.

khaeru commented 4 years ago

@hlinero note in the right sidebar that I added this to the GitHub ‘project’ named “iTEM Open Data”. Please do this for all related issues/PRs.

khaeru commented 4 years ago

Updated title and description slightly to be, at least, not inaccurate. In the future this should be done as the code evolves.