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

Features/restructure network #791

Merged
merged 28 commits into from
Nov 23, 2021
Merged

Features/restructure network #791

merged 28 commits into from
Nov 23, 2021

Conversation

p-snft
Copy link
Member

@p-snft p-snft commented Nov 5, 2021

Puts a clean structure to the submodules, i.e. components and blocks are now located next to each other (implements #588).

Summary:

  • parts of the network are now clearly structured into buses, components, and flows.
  • public and private API should be more distinguished now ('_' signifies private, public API should be exposed using __all__)
  • started API for distinct Flow, InvestmentFlow, and NonConvexFlow
  • experimental stuff is now sitting in submodules called experimental (replaces "custom")

Tasks:

  • Restructure
  • Adjust unit tests
  • Adjust documentation (at least the part that is tested)
  • Add new flow classes (NonConvexFlowand InvestmentFlow)
  • Add to WhatsNew

Known issues: A lot of things still need to be adjusted.
LP files for the tests are among them.
@pep8speaks
Copy link

pep8speaks commented Nov 5, 2021

Hello @p-snft! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-11-15 10:03:42 UTC

@p-snft
Copy link
Member Author

p-snft commented Nov 5, 2021

I am currently wondering if components, buses and flows should be available in solph directly or if categories should be provided. For example, do you from oemof.solph import GenericStorage or from oemof.solph.components import GenericStorage?

Little update: Currently, I allow both. Experimental stuff has to be imported from the specific submodule.

p-snft and others added 23 commits November 5, 2021 11:11
"Modules should explicitly declare the names in
their public API using the __all__ attribute."
https://www.python.org/dev/peps/pep-0008/#public-and-internal-interfaces
Unitl this commit only PR into dev or master trigger the workflow. Now
all PR will trigger the workflow.
This file should not be there, it's not part of the API.
There is now InvestmentFlow and NonConvexFlow. The latter takes all
argments that were previously hidden in the NonConvex object. The
InvestmentFlow currently only is a thin wrapper without any logic.
@p-snft
Copy link
Member Author

p-snft commented Nov 9, 2021

I am not sure about the API of the InvestmentFlow, but as this might be added later, I will now convert this draft to a full PR.

@p-snft p-snft marked this pull request as ready for review November 9, 2021 16:06
@p-snft p-snft requested a review from uvchik November 15, 2021 10:00
Copy link
Member

@uvchik uvchik left a comment

Choose a reason for hiding this comment

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

For me it looks fine, even though I may have missed something due to the high number of changes. But as it is not a merge into a stable branch we may have the chance to bring small issues in line, if there are any 😄

@p-snft p-snft merged commit 741f51c into v0.5 Nov 23, 2021
@p-snft p-snft deleted the features/restructure_network branch November 23, 2021 07:37
@fwitte fwitte mentioned this pull request Dec 1, 2021
15 tasks
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.

None yet

3 participants