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

Updating Westeros tutorials and adding new ones #232

Merged
merged 19 commits into from
Oct 2, 2019

Conversation

behnam-zakeri
Copy link
Contributor

@behnam-zakeri behnam-zakeri commented Sep 20, 2019

This PR updates the existing Westeros tutorials and adds a new tutorial for improving the learning experience of the users.

This PR does the following:

  • It updates the text and improves the readabilaty of the Westeros tutorials, including Westros baseline, emission bound, emission tax, firm capacity, and flexible generation.
  • It adds a new Westeros tutorial for showing how to add sub-annual time steps and assessing the impact of seasonality in a MESSAGE model

Doesn't need a test and documentation
Release notes updated.

@behnam-zakeri behnam-zakeri force-pushed the tutorials_update branch 2 times, most recently from cb2314f to a8902cd Compare September 24, 2019 12:55
RELEASE_NOTES.md Outdated Show resolved Hide resolved
Copy link
Member

@khaeru khaeru left a comment

Choose a reason for hiding this comment

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

Thanks @behnam-zakeri for these improvements to the existing tutorials and the new one, which sets a high standard for clarity that I think learners will appreciate. I added some minor comments inline.

One additional change I spotted in westeros_baseline.ipynb that is not part of a line you changed: “The full framework documentation is available at https://messageix.iiasa.ac.at” — should be https://message.iiasa.ac.at.

tutorial/westeros/westeros_baseline.ipynb Outdated Show resolved Hide resolved
tutorial/westeros/westeros_baseline.ipynb Show resolved Hide resolved
tutorial/westeros/westeros_baseline.ipynb Outdated Show resolved Hide resolved
tutorial/westeros/westeros_baseline.ipynb Outdated Show resolved Hide resolved
tutorial/westeros/westeros_emissions_taxes.ipynb Outdated Show resolved Hide resolved
tutorial/README.rst Show resolved Hide resolved
tutorial/westeros/westeros_seasonality.ipynb Outdated Show resolved Hide resolved
tutorial/westeros/westeros_seasonality.ipynb Show resolved Hide resolved
tutorial/westeros/westeros_seasonality.ipynb Outdated Show resolved Hide resolved
Copy link
Contributor Author

@behnam-zakeri behnam-zakeri left a comment

Choose a reason for hiding this comment

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

Thanks a lot @khaeru for a thorough review and constructive feedback. I applied the changes you mentioned.

@behnam-zakeri
Copy link
Contributor Author

behnam-zakeri commented Oct 1, 2019

One additional change I spotted in westeros_baseline.ipynb that is not part of a line you changed: “The full framework documentation is available at https://messageix.iiasa.ac.at” — should be https://message.iiasa.ac.at.

Thanks @khaeru for noticing this. Corrected.

@khaeru
Copy link
Member

khaeru commented Oct 2, 2019

Okay, will merge this now.

As discussed in today's MESSAGE meeting, the following PRs made were made against the behnam-zakeri:tutorials_update branch. After this PR merges that branch, it will be deleted. These can be re-opened as PRs agains iiasa:master; either individually or as a combined branch:

@khaeru khaeru merged commit c8140cf into iiasa:master Oct 2, 2019
@khaeru
Copy link
Member

khaeru commented Oct 2, 2019

Thanks @behnam-zakeri!

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.

2 participants