Skip to content

Conversation

@cyiallou
Copy link
Contributor

@cyiallou cyiallou commented Jul 11, 2025

  • Update the introduction in the instructions.
  • Utilise the updated MicrogridConfig class to simplify the toml
    file loading.
  • Utilise the new state_analysis module for detecting and analysing
    component state transitions and alerts from reporting data. This
    simplifies the notebook by removing a now obsolete code.

Copilot AI review requested due to automatic review settings July 11, 2025 15:58
@cyiallou cyiallou requested a review from a team as a code owner July 11, 2025 15:58
@cyiallou cyiallou added the cmd:skip-release-notes It is not necessary to update release notes for this PR label Jul 11, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot wasn't able to review any files in this pull request.

@github-actions github-actions bot added the part:docs Affects the documentation label Jul 11, 2025
@cyiallou cyiallou requested a review from cwasicki July 11, 2025 16:00
@cyiallou
Copy link
Contributor Author

please also double check the German translation

@cwasicki
Copy link
Contributor

There is still quite a lot of code in the notebook, which would be better moved to the lib. Also we should aim for having only a single library (notebooks) that the users need to worry about, not 3.

@cwasicki
Copy link
Contributor

please also double check the German translation

Translation LGTM

@cyiallou
Copy link
Contributor Author

There is still quite a lot of code in the notebook, which would be better moved to the lib.

There is some code that will be deleted once this PR is merged frequenz-floss/frequenz-reporting-python#58

I will review other parts of the notebook to see what can be moved to the library. Do you see specific parts that should be moved already?

Also we should aim for having only a single library (notebooks) that the users need to worry about, not 3.

You are right. Just realised that client-common can be removed from the notebook because it is already installed as a sub-dependency. The frequenz-reporting-python is needed because I am using the code from frequenz-floss/frequenz-reporting-python#58 so this dependency can also be dropped if we move most of the frequenz/reporting/_reporting.py module (all code except cumulative_energy()) in lib-notebooks. What do you think?

@cwasicki
Copy link
Contributor

From my side it's fine to do this later. We should aim for notebooks that have just couple of lines for interaction with the user and display, and everything else in a python module.

@cyiallou cyiallou force-pushed the docs/update-notebook-instructions branch from 428cccd to 7d87593 Compare July 23, 2025 16:04
- Update the introduction in the instructions.
- Utilise the updated `MicrogridConfig` class to simplify the toml
  file loading.
- Utilise the new `state_analysis` module for detecting and analysing
  component state transitions and alerts from reporting data. This
  simplifies the notebook by removing a now obsolete code.

Signed-off-by: cyiallou - Costas <[email protected]>

s
@cyiallou cyiallou force-pushed the docs/update-notebook-instructions branch from 7d87593 to e68922d Compare July 23, 2025 16:20
@cyiallou cyiallou changed the title Update Alert Notebook instructions Update Alert Notebook Jul 23, 2025
@cyiallou
Copy link
Contributor Author

Updated @cwasicki

@cyiallou cyiallou mentioned this pull request Jul 23, 2025
@cyiallou cyiallou added this pull request to the merge queue Jul 23, 2025
Merged via the queue into frequenz-floss:v0.x.x with commit 8c15770 Jul 23, 2025
5 checks passed
@cyiallou cyiallou deleted the docs/update-notebook-instructions branch July 23, 2025 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cmd:skip-release-notes It is not necessary to update release notes for this PR part:docs Affects the documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants