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 a module for implementing (sub-annual) time slices #852

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

glatterf42
Copy link
Member

This PR supersedes iiasa/message-ix-models#197 as @khaeru suggested this tool might be better placed in message_ix.

@behnam-zakeri Please take a look at the changes. Several things come to mind before merging this:

  • Use the existing logging functionality instead of printing to stdout
  • Write a bit of documentation on how to use this tool (e.g. via a README.rst as was done for add_year)
  • Write tests for this new functionality.

How to review

Check the above boxes plus:

  • Read the diff and note that the CI checks all pass.
  • Run a specific code snippet or command and check the output.
  • Build the documentation and look at a certain page.
  • Ensure that changes/additions are self-documenting, i.e. that another
    developer (someone like the reviewer) will be able to understand what the code
    does in the future.

PR checklist

  • Continuous integration checks all ✅
  • Add or expand tests; coverage checks both ✅
  • Add, expand, or update documentation.
  • Update release notes.

Copy link

codecov bot commented Jun 14, 2024

Codecov Report

Attention: Patch coverage is 0% with 255 lines in your changes missing coverage. Please review.

Project coverage is 90.3%. Comparing base (1125579) to head (baeb7f8).

Additional details and impacted files
@@           Coverage Diff           @@
##            main    #852     +/-   ##
=======================================
- Coverage   95.6%   90.3%   -5.4%     
=======================================
  Files         46      47      +1     
  Lines       4333    4588    +255     
=======================================
  Hits        4143    4143             
- Misses       190     445    +255     
Files Coverage Δ
message_ix/tools/add_timeslice/add_timeslice.py 0.0% <0.0%> (ø)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enh New features & functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants