Skip to content

Include plot in introductory information to illustrate concept#177

Merged
sadielbartholomew merged 8 commits intoGreenScheduler:mainfrom
sadielbartholomew:minor-docs-updates
Jan 21, 2026
Merged

Include plot in introductory information to illustrate concept#177
sadielbartholomew merged 8 commits intoGreenScheduler:mainfrom
sadielbartholomew:minor-docs-updates

Conversation

@sadielbartholomew
Copy link
Copy Markdown
Member

After #171 updated the plot from POC form, it is nice to showcase it in the introductory information, with a brief note about how the optimal window is where the area under the curve is minimised, as a neat visual demonstration of the overall concept of carbon intensity minimisation which is the key concept for CATS. Namely:

  • include in README;
  • include in the 'quickstart' page of the docs (here the 'intro' is more about a summary and background, rather than any nitty-gritty, so best in 'quickstart' instead IMO).

Also moves the GIF to the docs.source/_static directory, updating any links to that path (just two), since all other images and media are there, as with the example plot which I generated and committed. Alternatively we could have a root-level 'media' directory, but with minimal media I don't think this is necessary.

@sadielbartholomew sadielbartholomew added the documentation Improvements or additions to documentation label Dec 3, 2025
@sadielbartholomew sadielbartholomew requested a review from a team January 16, 2026 15:22
Copy link
Copy Markdown
Collaborator

@andreww andreww left a comment

Choose a reason for hiding this comment

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

All looks good to me. I wonder if it's worth including the output (to the terminal and/or the plot) in the examples in the quickstart but that's orthogonal to this change. Feel free to merge.

Comment thread docs/source/quickstart.rst
@andreww
Copy link
Copy Markdown
Collaborator

andreww commented Jan 20, 2026

Strikes me that we don't have "rules" written down anywhere about who should merge PRs, just a requirement to have them reviewed. I'm happy for @sadielbartholomew to hit merge for this but that's something we should probably document in contributing...

@sadielbartholomew
Copy link
Copy Markdown
Member Author

Thanks for the review, @andreww. All good points - I am going to add a few commits to address them.

Strikes me that we don't have "rules" written down anywhere about who should merge PRs, just a requirement to have them reviewed.

Yes it would be good to have a rule-of-thumb, even if it isn't written down. I personally like the rule of 'the merger is the PR author' (who can merge once they have at least one approval review or as per whatever review rules) since it's simple and keeps responsibility with those already considering a given PR. So that would be my suggestion.

@sadielbartholomew
Copy link
Copy Markdown
Member Author

OK have updated in line with Andrew's good feedback - ready for re-review to sanity check the updates, else a fresh review.

Copy link
Copy Markdown
Collaborator

@andreww andreww left a comment

Choose a reason for hiding this comment

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

Everything looks good apart from the plot image in the rendered quickstart document (https://cats--177.org.readthedocs.build/en/177/quickstart.html) is showing (for me) as text and doesn't show the actual plot. I think this is a markdown v's restructured text issue. I'll take a look and see if I can quickly figure out the fix.

Comment thread docs/source/quickstart.rst Outdated
Use restructured text not markdown
@andreww
Copy link
Copy Markdown
Collaborator

andreww commented Jan 21, 2026

All good @sadielbartholomew - happy for you to hit merge.

@sadielbartholomew sadielbartholomew merged commit ec159b6 into GreenScheduler:main Jan 21, 2026
4 checks passed
@sadielbartholomew sadielbartholomew deleted the minor-docs-updates branch January 21, 2026 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants