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

Add information to namedtuple tutorial #1095

Open
wants to merge 1 commit into
base: feature/data_analysis_tutorial
Choose a base branch
from

Conversation

Parth-Vachhani
Copy link

Changes made in feature/data_analysis_tutorial

@p-snft p-snft mentioned this pull request Aug 2, 2024
@Parth-Vachhani Parth-Vachhani deleted the feature/data_analysis_tutorial branch August 7, 2024 09:34
@Parth-Vachhani Parth-Vachhani restored the feature/data_analysis_tutorial branch August 19, 2024 12:40
Copy link
Member

@jokochems jokochems 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 for opening this pull request. However, I think, it requires some adaptions:

  • Most importantly, we should have a cleaned version without any execution output.
  • Besides, I think some changes are not necessary, such as spreading markdown explanations across multiple cells.

@@ -12,7 +12,7 @@
"cell_type": "markdown",
"metadata": {},
"source": [
"We want to model a group of buildings, each with demands for electricity and space heating.\n",
"We want to model a group of buildings, each with demands for electricity and space heating. \n",
Copy link
Member

Choose a reason for hiding this comment

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

In my view, the space is not needed as there is the line break.

Comment on lines +25 to +30
]
},
{
"cell_type": "markdown",
"metadata": {},
"source": [
Copy link
Member

Choose a reason for hiding this comment

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

I don't see the necessity to add another markdown cell. Instead, I'd suggest to add the text to the above cell.

Comment on lines -32 to +65
"execution_count": null,
"execution_count": 2,
Copy link
Member

Choose a reason for hiding this comment

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

We should host a cleaned up version, not showing any execution results.

"source": [
"import networkx as nx\n",
"from oemof.network.graph import create_nx_graph\n",
"\n",
"graph = create_nx_graph(energy_system)\n",
"\n",
"nx.draw(graph, with_labels=True, font_size=8)"
"nx.draw(graph, with_labels=True, font_size=7)"
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reasoning to alter the font_size attribute?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants