Skip to content

Conversation

@khaeru
Copy link
Member

@khaeru khaeru commented Jul 19, 2025

Removal of this argument in #581 broke message_ix and message-ix-models main branches, as evidenced by scheduled CI, for instance here:

FAILED message_ix/tests/util/test_gams_io.py::test_add_default_data_to_container_data_list[ixmp4] - TypeError: GAMSModel.__init__() takes 1 positional argument but 2 were given
= 220 failed, 41 passed, 5 xfailed, 1 xpassed, 4 warnings, 114 errors in 60.77s (0:01:00) =

Related:

  • Remove types.GamsModelInitKwargs.name_.
  • Use GamsModelInitKwargs for GAMSModel only. Other functions, like ixmp.model.get_model() and Scenario.solve(), must be usable with any subclass of ixmp.model.base.Model. If such subclasses are created in the future (see for instance Migrating away from GAMS message_ix#879 (comment)) they may have different arguments than GAMSModel.

Also:

  • Reduce log verbosity in .base.Model.initialize_items(). Changes to this method in Enable strict mypy settings #581 resulted in spurious messages like:
    Existing index names of 'renewable_potential' ('node', 'commodity', 'grade', 'level', 'year') do not match ()
    
    These are spurious because an argument value of idx_names=() used when creating a parameter is automatically replaced with the same value as idx_sets, resulting in the values shown; so there is no difference.

How to review

  • Read the diff and note that the CI checks all pass.
  • Run the message_ix test suite with the code on this branch.

PR checklist

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

- Don't type the parent class with these subclass-specific args.
- Don't type get_model() or Scenario.solve() with these class-specific
  args.
- Restore GAMSModel.__init__(..., name_=...) positional arg.
- Remove name_ from GAMSModelInitKwargs.
@khaeru khaeru self-assigned this Jul 19, 2025
@khaeru khaeru added bug ci Continuous integration labels Jul 19, 2025
@codecov
Copy link

codecov bot commented Jul 19, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.1%. Comparing base (dc208c8) to head (f0e0897).
⚠️ Report is 24 commits behind head on main.

Additional details and impacted files
@@          Coverage Diff          @@
##            main    #587   +/-   ##
=====================================
  Coverage   98.0%   98.1%           
=====================================
  Files         51      51           
  Lines       6260    6262    +2     
=====================================
+ Hits        6141    6145    +4     
+ Misses       119     117    -2     
Files with missing lines Coverage Δ
ixmp/core/scenario.py 99.2% <ø> (ø)
ixmp/model/__init__.py 100.0% <100.0%> (ø)
ixmp/model/base.py 100.0% <100.0%> (ø)
ixmp/model/gams.py 100.0% <100.0%> (ø)
ixmp/tests/test_model.py 100.0% <100.0%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

khaeru added 3 commits July 19, 2025 18:18
Don't log for missing/unset idx_names, which are replaced in the backend
with the same values as idx_sets.
@khaeru khaeru force-pushed the fix/gamsmodel-init branch from 412ea30 to 12a5437 Compare July 19, 2025 16:21
khaeru added 2 commits July 19, 2025 18:40
- mypy v1.16 → v1.17
- ruff v0.11.13 → v0.12.4
  - Replace legacy "ruff" hook with "ruff-check".
@khaeru khaeru mentioned this pull request Jul 19, 2025
4 tasks
@khaeru khaeru merged commit 1730afd into main Jul 19, 2025
21 checks passed
@khaeru khaeru deleted the fix/gamsmodel-init branch July 19, 2025 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug ci Continuous integration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant