iiasa / message_ix

The integrated assessment and energy systems model MESSAGEix
https://docs.messageix.org
Apache License 2.0
111 stars 149 forks source link

Housekeeping for 2024-W13 #809

Closed khaeru closed 3 months ago

khaeru commented 3 months ago

How to review

Read the diff and note that the CI checks all pass.

PR checklist

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 95.4%. Comparing base (e33ae53) to head (f4cbe57).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #809 +/- ## ===================================== Coverage 95.4% 95.4% ===================================== Files 46 46 Lines 4351 4354 +3 ===================================== + Hits 4153 4156 +3 Misses 198 198 ``` | [Files](https://app.codecov.io/gh/iiasa/message_ix/pull/809?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=iiasa) | Coverage Δ | | |---|---|---| | [message\_ix/core.py](https://app.codecov.io/gh/iiasa/message_ix/pull/809?src=pr&el=tree&filepath=message_ix%2Fcore.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=iiasa#diff-bWVzc2FnZV9peC9jb3JlLnB5) | `97.7% <100.0%> (+<0.1%)` | :arrow_up: | | [message\_ix/macro.py](https://app.codecov.io/gh/iiasa/message_ix/pull/809?src=pr&el=tree&filepath=message_ix%2Fmacro.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=iiasa#diff-bWVzc2FnZV9peC9tYWNyby5weQ==) | `96.7% <100.0%> (ø)` | | | [message\_ix/report/operator.py](https://app.codecov.io/gh/iiasa/message_ix/pull/809?src=pr&el=tree&filepath=message_ix%2Freport%2Foperator.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=iiasa#diff-bWVzc2FnZV9peC9yZXBvcnQvb3BlcmF0b3IucHk=) | `97.2% <100.0%> (+<0.1%)` | :arrow_up: | | [message\_ix/report/pyam.py](https://app.codecov.io/gh/iiasa/message_ix/pull/809?src=pr&el=tree&filepath=message_ix%2Freport%2Fpyam.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=iiasa#diff-bWVzc2FnZV9peC9yZXBvcnQvcHlhbS5weQ==) | `100.0% <100.0%> (ø)` | | | [message\_ix/testing/\_\_init\_\_.py](https://app.codecov.io/gh/iiasa/message_ix/pull/809?src=pr&el=tree&filepath=message_ix%2Ftesting%2F__init__.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=iiasa#diff-bWVzc2FnZV9peC90ZXN0aW5nL19faW5pdF9fLnB5) | `99.6% <100.0%> (-0.1%)` | :arrow_down: | | [message\_ix/tests/test\_feature\_duration\_time.py](https://app.codecov.io/gh/iiasa/message_ix/pull/809?src=pr&el=tree&filepath=message_ix%2Ftests%2Ftest_feature_duration_time.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=iiasa#diff-bWVzc2FnZV9peC90ZXN0cy90ZXN0X2ZlYXR1cmVfZHVyYXRpb25fdGltZS5weQ==) | `100.0% <100.0%> (ø)` | | | [message\_ix/tools/add\_year/\_\_init\_\_.py](https://app.codecov.io/gh/iiasa/message_ix/pull/809?src=pr&el=tree&filepath=message_ix%2Ftools%2Fadd_year%2F__init__.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=iiasa#diff-bWVzc2FnZV9peC90b29scy9hZGRfeWVhci9fX2luaXRfXy5weQ==) | `66.2% <100.0%> (ø)` | | | [message\_ix/util/\_\_init\_\_.py](https://app.codecov.io/gh/iiasa/message_ix/pull/809?src=pr&el=tree&filepath=message_ix%2Futil%2F__init__.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=iiasa#diff-bWVzc2FnZV9peC91dGlsL19faW5pdF9fLnB5) | `98.7% <ø> (ø)` | |
khaeru commented 3 months ago

Do you want to fix them here or should this be a new PR?

I think these should perhaps be PRs upstream. This is basically what we tried to solve in khaeru/genno#94 and khaeru/genno#108, and I now have a different idea of how to resolve these issues.

Previously (those above PRs) we tried to use the intersphinx_mapping to force lookups to resolve correctly. I don't know if this does/can work.

Instead, I think we can change some references like this: https://github.com/iiasa/ixmp/blob/5713fc14c0206c664405e4dd59576808b65832b9/ixmp/core/timeseries.py#L326 to this:

… :meth:`.Platform.add_timeslice`

Why should that hopefully work?

  1. Within ixmp's docs: a. ".add_timeslice" as the reference target, with the prefixing ".", means "a method named add_timeslice, anywhere within this Sphinx project". This is resolved with "ixmp.Platform.add_timeslice", because that's the only internal reference target that (a) is a method and (b) has that name. In other words, the "." prefix does not trigger that matching behaviour inclusive of intersphinx inventories or external targets. I think there's no easy way to force intersphinx to do so. b. ".Platform.add_timeslice" resolves to the same target but is more verbose, so often we write (a).
  2. Within message_ix's docs, if the docstring for ixmp.TimeSeries.add_timeseries is shown as part of the subclass message_ix.Scenario: a. ".add_timeslice" as the reference target has the same meaning. But given the "." prefix, Sphinx does not find any (a) method (b) named "add_timelice" within the same project, so the reference is unresolved and we see a warning. b. ".Platform.add_timeslice" as the target together with the new genno.compat.sphinx.rewrite_refs extension, gets rewritten to "ixmp.Platform.add_timeslice", and this full ref is found exactly in the intersphinx inventory.

This will be a little awkward because we basically will need to:

…however, I think would be pretty quick to do in a couple of follow-up PRs. Also not very urgent; can be done closer to the next release.