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

Update Westeros multinode tutorial for hints #798

Merged
merged 6 commits into from
Mar 20, 2024

Conversation

behnam-zakeri
Copy link
Contributor

@behnam-zakeri behnam-zakeri commented Mar 12, 2024

This PR updates the multinode tutorial for two reasons:

How to review

Please run Westeros multinode tutorial and make sure the hints provided at the end will work. You can copy-paste those code-based hints at the end of the tutorial and solve the illustrated scenario.

PR checklist

  • Continuous integration checks all ✅
  • [ ] Add or expand tests; coverage checks both ✅ Just adding hints to one tutorial
  • Add, expand, or update documentation.
  • Update release notes.

@behnam-zakeri behnam-zakeri marked this pull request as draft March 12, 2024 13:06
@behnam-zakeri behnam-zakeri self-assigned this Mar 12, 2024
@behnam-zakeri behnam-zakeri added the docs Documentation label Mar 12, 2024
@behnam-zakeri
Copy link
Contributor Author

@glatterf42 I added the code-based hints at the end of the tutorial. These can be opened by click. I couldn't'd wrap up the code block using "```", so the code block does not look optimal when the hint is clicked. Maybe you could help me with that.

Copy link

codecov bot commented Mar 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.4%. Comparing base (023b91a) to head (06b0cd6).

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

Copy link
Member

@glatterf42 glatterf42 left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks :)

The tutorial tests may be failing, but that's unrelated to this PR.

Just to be sure: even after reformatting the code (for which I used markdown and removed the html <code> directive), the code can only be copied by editing the cell, not immediately from the rendered output. Is that fine with you?

@glatterf42 glatterf42 marked this pull request as ready for review March 13, 2024 11:06
@meng25meng

This comment was marked as off-topic.

@behnam-zakeri
Copy link
Contributor Author

Looks good to me, thanks :)

The tutorial tests may be failing, but that's unrelated to this PR.

Just to be sure: even after reformatting the code (for which I used markdown and removed the html <code> directive), the code can only be copied by editing the cell, not immediately from the rendered output. Is that fine with you?

Thanks @glatterf42 for reviewing this. Yes, the code needs to be copied to a new cell and be run. As far as I know it cannot be hidden in markdown cell and later be run as a code block.

@glatterf42

This comment was marked as off-topic.

@meng25meng

This comment was marked as off-topic.

@glatterf42
Copy link
Member

@behnam-zakeri if the new name suits you, this PR is good to go :)

Mypy v1.8.0 → v1.9.0
Ruff v0.3.0 → v0.3.2
pre-commit/action v3.0.0 → v3.0.1
@behnam-zakeri
Copy link
Contributor Author

Thanks, @glatterf42, the name sounds more intuitive and representative of the content. I believe this is ready to be merged.

@behnam-zakeri behnam-zakeri merged commit 99422f4 into iiasa:main Mar 20, 2024
24 checks passed
glatterf42 added a commit to behnam-zakeri/message_ix that referenced this pull request May 17, 2024
…a#798)

* Update multinode tutorial after resolving issue iiasa#601

* Extend multinode tutorial to include hints for questions

* Add release notes iiasa#798

* Apply ruff and format hint code block

* Rename tutorial to mention energy trade

* Update mypy, ruff, pre-commit config

Mypy v1.8.0 → v1.9.0
Ruff v0.3.0 → v0.3.2
pre-commit/action v3.0.0 → v3.0.1

---------

Co-authored-by: Fridolin Glatter <glatter@iiasa.ac.at>
Co-authored-by: Paul Natsuo Kishimoto <mail@paul.kishimoto.name>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants