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

Add .tools.res_marg #277

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Add .tools.res_marg #277

wants to merge 4 commits into from

Conversation

khaeru
Copy link
Member

@khaeru khaeru commented Jan 13, 2025

Partly addresses #275.

This PR does three things:

(1) Copies from message_data ssp_dev branch:

  • File message_data/scenario_generation/reserve_margin/res_marg.py
  • Last modified by commit iiasa/message_data@f3efc8b104044676434695aa461d26a7b20e5cd7
  • message_single_country SSP_Dev_2023 branch contains an identical file.

(2) Adjusts the copied code to confirm, in a minimal way, to the code style:

  • Docstring is correctly formatted.
  • Modules and functions appear in the documentation.
  • Code is type hinted.
  • Tests exist, even if they are incomplete/fail.

The above two steps are mandatory for PRs related to #275.
Then, optionally:

(3) Improves the code.

  • Add missing tests.
  • Re-use existing utility code in message-ix-models, e.g. the CLI.

How to review

TBA

PR checklist

  • Continuous integration checks all ✅
  • Add or expand tests; coverage checks both ✅
  • Add, expand, or update documentation.
  • Update doc/whatsnew.

- Copied from message_data `ssp_dev` branch
  - File message_data/scenario_generation/reserve_margin/res_marg.py
  - Last modified by commit f3efc8b104044676434695aa461d26a7b20e5cd7:
    Author: FRICKO Oliver <[email protected]>
    Date:   Wed Nov 27 13:48:26 2024 +0100

    Update reserve_margin script to remove print statement
- message_single_country `SSP_Dev_2023` branch contains an identical
  file.
@khaeru khaeru added the enh New features or functionality label Jan 13, 2025
@khaeru khaeru self-assigned this Jan 13, 2025
- Format docstring according to code style, rewrap.
- Add type hints.
- Ensure the module appears in the docs.
- Add (incomplete) tests for all parts of the code.
Copy link

codecov bot commented Jan 13, 2025

Codecov Report

Attention: Patch coverage is 50.00000% with 15 lines in your changes missing coverage. Please review.

Project coverage is 65.5%. Comparing base (dbd783e) to head (d8751c5).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
message_ix_models/tools/res_marg.py 35.0% 13 Missing ⚠️
message_ix_models/tests/tools/test_res_marg.py 80.0% 2 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (dbd783e) and HEAD (d8751c5). Click for more details.

HEAD has 59 uploads less than BASE
Flag BASE (dbd783e) HEAD (d8751c5)
65 6
Additional details and impacted files
@@           Coverage Diff            @@
##            main    #277      +/-   ##
========================================
- Coverage   77.6%   65.5%   -12.1%     
========================================
  Files        211     213       +2     
  Lines      16079   16109      +30     
========================================
- Hits       12481   10567    -1914     
- Misses      3598    5542    +1944     
Files with missing lines Coverage Δ
message_ix_models/cli.py 93.4% <ø> (ø)
message_ix_models/tests/tools/test_res_marg.py 80.0% <80.0%> (ø)
message_ix_models/tools/res_marg.py 35.0% <35.0%> (ø)

... and 48 files with indirect coverage changes

- Integrate with existing `mix-models` CLI.
- Remove duplicated code for model/scenario/version arguments, loading
  platform and scenario, etc.
- Simplify tests.
@khaeru khaeru mentioned this pull request Jan 13, 2025
3 tasks
khaeru added a commit that referenced this pull request Jan 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enh New features or functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant