-
Notifications
You must be signed in to change notification settings - Fork 167
Separate models.py into submodules #972
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
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
1efe903
Change mode of core.py to match other files
khaeru d7acd06
Add model/README
khaeru 204352f
Move .macro → .macro.calibrate
khaeru f70eb6d
Split .models to 4 submodules
khaeru 8615910
Adjust 1st-party imports of .models contents
khaeru 73aaa8b
Allow Scenario(…, scheme="MESSAGE-MACRO")
khaeru 6baab9e
Rearrange and expand tests for #972
khaeru df1bfa9
Update docs, release notes for #972
khaeru File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm looking at the docs build and I'm surprised to see lots and lots of errors. Most of them already existed for the
latestbuild, though, it seems. What's new is thatdoc/macro.rstcomplains about not being able to import various things frommessage_ix.macro. I'm guessing they moved somewhere (too many to list, but starting here in the build logs).I also notice that there is an actual error about the table that contains
PRICE_COMMODITYinmodel_core.rst, which is likely just imported from the GAMS file. This seems to be malformed, so is not rendered in our docs. While this error isn't new, it would be nice to fix it to have our docs be complete again. The error says "Bottom/header table border does not match top border.", which sounds like we just have a mismatch in the number of=used for the borders.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doc/install-adv.rstanddoc/reporting.rstalso complain about missing reference targets, as well as theRELEASE_NOTES.rst, curiously.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I'll fix up these examples you mention. It would be good to have someone do a more systematic sweep, at some point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This happened in #451: https://github.com/iiasa/message_ix/blame/cc014c24c7084f252f3827a86187a0ad467155a5/message_ix/model/MESSAGE/model_core.gms#L150
I've changed it to a ReST
list-table, which makes it easier to edit.That file is now
message_ix.macro.calibrate. Adjusted.Like a lot of Sphinx packages, the intersphinx inventory for JPype1 does not actually contain a target for the top-level module:
I added it to
reference_aliasesin doc/conf.py alongside the other such cases.This one looked like:
message_ix/doc/install-adv.rst
Line 233 in cc014c2
The shorthand whereby
.IXMP4Backendwill find a fully-qualified name (FQN) likeixmp.backend.ixmp4.IXMP4Backendonly functions within the same project, not across projects via intersphinx. I replaced with the FQN.Most of these cases occur with references that work/are valid within the genno docs, but are not properly resolved when those docs are rendered within the context of the message_ix docs. Some of those might be resolvable by copying some of the genno doc/conf.py settings. I'll omit those for now.
Added the appropriate targets.
Same as above.
This was trying to use :meth: to reference a :class:. Fixed.
This happens when things are removed. I resolve these by using the :py: role, which looks code-like but no longer tries to link to anything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this already looks a lot better. And agreed that a more systematic sweep would be useful, but out of scope here :)