Improve and adapt notebooks with latest environment and optional wildcards#107
Conversation
|
Hi @doneachh, I've updated the |
|
@huyhoang-mike hydrogen_financing.ipynb, hydrogen_market.ipynb, network_analysis_sec.ipynb do work perfectly fine! When trying to run network_analysis.ipynb with a sector coupled network, i get some errors. Can you try to adjust this notebook, that you can analyze sector coupled as well as electricity only networks? :) |
|
@ekatef can you merge the two PRs? :) thank you! |
|
Hi @doneachh, I would like to just add some heads-up:
|
@huyhoang-mike exactly. Feel free to go ahead and tag Katja as soon as the PR is ready to merge. |
|
Hi @ekatef, this PR has been completely finished. It would be nice if you can review it :) |
|
Hello @doneachh and @huyhoang-mike! @huyhoang-mike it looks like there may be still something wrong with
I see that cells are numbered in |
|
As an additional technical comment, the implementation includes quite sophisticated system of searching files including management of wildcards. I wonder if it may be a good idea to store those helpers scripts in a dedicated file and import them into all the notebooks where they can be needed? It would make the notebooks much lighter and facilitate maintenance significantly thanks to reducing duplication @huyhoang-mike would you be interested to work on that? |
|
@ekatef yes, I'm totally up for this topic. Thanks for your suggestion! Modularizing the network searching process would make the notebook way simpler and more approachable for these tutorials. I will add a helper script in the next PR :) |
Hey @huyhoang-mike, happy you like the idea! I think it could be a great next step. In the meantime, would you mind to add fixes to |
|
Hi @ekatef, thanks a lot for your comprehensive review! I have discussed with @doneachh, and solved the mentioned concerns in the #107 (comment). Here is the summary:
If everything is fine now, I will proceed with the network searching helper in the next PR. Thanks for your wonderful collaboration :) |
|
@huyhoang-mike @ekatef Hi! @huyhoang-mike are the notebooks running for sector coupled + electricity only results of the newest main? |
|
Feel free to contact me in case I can assist :) |
|
Hey @huyhoang-mike apologies have completely missed your response in December (for future, please feel free to ping us if you don't get answer on the revisions during a few days). I think For |
* added my name to README.md * added my name to README.md
* Added my name to README file
|
Hi @ekatef, I’ve rebased and then rerun the two notebooks you mentioned and fixed the execution‑count issue. While testing the Let me know if this works for you :) Then we can go ahead and merge it. |
|
Hi @doneachh, the |
Hey @huyhoang-mike, thank you so much for the update. I think the changes make perfect sense, and the contribution looks great. I think we are ready to merge unless @doneachh has any further comments. |
|
A maintenance note: a default GitHib render seems be incorrect for rebase. The commits which are still in main are still highlighted in changes, while git diff shows that PR modifies those and only those files which it's intended to modify (checked locally with Sublime Merge):
|
@ekatef from my side no further comments :) |
|
Merged! 🎉 🎉 🎉 |
|
@ekatef My pleasure 🎉 Thanks a lot for your reviewing and helpful feedback. |

This PR resolves the following issues:
hydrogen_market.ipynb,hydrogen_financing.ipynb, andnetwork_analysis_sec.ipynb.