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

Dac tutorial message models #807

Closed
wants to merge 3 commits into from
Closed

Conversation

gidden
Copy link
Member

@gidden gidden commented Mar 21, 2024

This is a fresh PR where we will implement DACCS related tutorials. It depends on iiasa/message-ix-models#158 where additional features for implementing DACCS in scenarios are developed.

This supercedes #793 and #806

FYI @ywpratama

Required: write a single sentence that describes the changes made by this PR.

How to review

Required: describe specific things that reviewer(s) must do, in order to ensure that the PR achieves its goal.
If no review is required, write “No review:” and describe why.

PR checklist

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

@@ -3,17 +3,19 @@
from functools import partial

from message_ix import Scenario
from message_ix.report import Key, Reporter, operator
from message_ix.report import Key, Reporter, computations
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI I took this from @ywpratama's active PR. Were there recent changes to the core Reporter api? (including file name changes?)

I would not expect all of the change lines here based on what is in https://github.com/iiasa/message_ix/pull/806/files#diff-c26536ce84986d4689980ad63dd90f92b8012bdfcb3bd55ed3df380f71d0c56a

I'll note also this tutorial ran for me locally, so I'm confused if message_ix.report not longer has a computations. FYI @glatterf42

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, there were changes in v3.8.0 to this exact module. See our release notes. You should be using from message_ix.report import operator now instead of ... computations.

Copy link

codecov bot commented Mar 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.4%. Comparing base (99422f4) to head (f04e68a).
Report is 1 commits behind head on main.

Additional details and impacted files
@@          Coverage Diff          @@
##            main    #807   +/-   ##
=====================================
  Coverage   95.4%   95.4%           
=====================================
  Files         46      46           
  Lines       4351    4351           
=====================================
  Hits        4153    4153           
  Misses       198     198           
Files Coverage Δ
message_ix/util/tutorial.py 100.0% <100.0%> (ø)

@gidden gidden mentioned this pull request Mar 21, 2024
4 tasks
@glatterf42
Copy link
Member

Thanks for keeping this work up-to-date, @gidden :)
However, I'm not sure we should be adding tutorials here that depend on code in message-ix-models. AFAIK, code in message-ix-models can rely on code in message_ix, but not the other way round. In theory, message_ix is a general tool and message-ix-models is about a specific application.
@khaeru please correct me if I'm wrong.

@glatterf42 glatterf42 added the enh New features & functionality label Mar 22, 2024
@khaeru
Copy link
Member

khaeru commented Mar 26, 2024

AFAIK, code in message-ix-models can rely on code in message_ix, but not the other way round. In theory, message_ix is a general tool and message-ix-models is about a specific application.

Yes, that's the order of our stack.

I do acknowledge that we have already a nice, inviting landing location in message-ix for new tutorials: there's a docs page, tests, etc. In message-ix-models, there are as yet no tutorials, so we lack corresponding items. However, we can pretty easily set that up by reusing or copying bits from message-ix. It would be great to have this as the first message-ix-models-specific tutorial.

@gidden
Copy link
Member Author

gidden commented Mar 28, 2024

Thanks both - would it be possible to get the skeleton of the docs pages in message-ix models and then we can add these there?

@gidden gidden mentioned this pull request Mar 28, 2024
4 tasks
@gidden
Copy link
Member Author

gidden commented Mar 28, 2024

Hi all - after further discussions with @ywpratma we decided to make his add_dac tool more generic and will open a different PR for that

@gidden gidden closed this Mar 28, 2024
@gidden gidden deleted the dac_tutorial_message_models branch March 28, 2024 12:51
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.

None yet

4 participants