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 network function #781

Closed
wants to merge 33 commits into from
Closed

Conversation

jessLryan
Copy link
Contributor

Closes #506

Changes proposed in this Pull Request

Checklist

  • [yes] Code changes are sufficiently documented; i.e. new functions contain docstrings and further explanations may be given in doc.
  • [yes] Unit tests for new features were added (if applicable).
  • [N/A] Newly introduced dependencies are added to environment.yaml, environment_docs.yaml and setup.py (if applicable).
  • [yes] A note for the release notes doc/release_notes.rst of the upcoming release is included.
  • [yes] I consent to the release of this PR's code under the MIT license.

jessLryan and others added 7 commits November 3, 2023 21:13
function to add the components of one network to another
Bug fixes and added unit test for adding a network with only bus components to another network. More tests needed.
added description of new method to release file
pre-commit-ci bot and others added 3 commits November 10, 2023 21:44
Copy link

codecov bot commented Nov 11, 2023

Codecov Report

Attention: Patch coverage is 71.42857% with 10 lines in your changes are missing coverage. Please review.

Project coverage is 78.78%. Comparing base (4298c23) to head (a1513f9).

❗ Current head a1513f9 differs from pull request most recent head 8fdf880. Consider uploading reports for the commit 8fdf880 to get more accurate results

Files Patch % Lines
pypsa/components.py 71.42% 5 Missing and 5 partials ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master     #781       +/-   ##
===========================================
+ Coverage   66.26%   78.78%   +12.51%     
===========================================
  Files          26       26               
  Lines        7066     7116       +50     
  Branches     1418     1567      +149     
===========================================
+ Hits         4682     5606      +924     
+ Misses       2102     1185      -917     
- Partials      282      325       +43     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

jessLryan and others added 4 commits November 29, 2023 18:30
Updated method to include additional inputs suggested by Fabian and error thrown if snapshots/snapshot weightings in the networks do not agree. Also added warning if components with IDs matching component in existing network are added - this warning does not apply to components of type LineType or TransformerType but duplicate components will still not be added in this case.

Updated static test to reflect these changes and added new tests for the case where the method returns a new network (inplace=false), and the case where time-varying data is added (with_time=True_
error from running locally
Copy link
Collaborator

@FabianHofmann FabianHofmann left a comment

Choose a reason for hiding this comment

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

before reviewing, I was thinking about a better naming. Perhaps import_from_network would be better aligning with the other import function names?

@jessLryan
Copy link
Contributor Author

before reviewing, I was thinking about a better naming. Perhaps import_from_network would be better aligning with the other import function names?

how about "add_components_from_network"? Could also change the current add function in components.py to "add_component" so that the distinction is clear?

@FabianHofmann
Copy link
Collaborator

before reviewing, I was thinking about a better naming. Perhaps import_from_network would be better aligning with the other import function names?

how about "add_components_from_network"?

sounds good

Could also change the current add function in components.py to "add_component" so that the distinction is clear?

which function do you mean exactly? I would not touch existing ones though...

@jessLryan
Copy link
Contributor Author

before reviewing, I was thinking about a better naming. Perhaps import_from_network would be better aligning with the other import function names?

how about "add_components_from_network"?

sounds good

okay great I'll do that :-)

Could also change the current add function in components.py to "add_component" so that the distinction is clear?

which function do you mean exactly? I would not touch existing ones though...

fair point

jessLryan and others added 2 commits November 30, 2023 08:56
renamed add_network function to add_components_from_network
also moved function to follow existing add function (for adding single components)
updated tests to match function rename
@jessLryan
Copy link
Contributor Author

@FabianHofmann what's the reason for the failing tests? I can't work it out from the output

@FabianHofmann
Copy link
Collaborator

I have no clue, I retriggered them

@jessLryan
Copy link
Contributor Author

@FabianHofmann can this now be merged?

@FabianHofmann
Copy link
Collaborator

Hey @jessLryan, I will review as soon as I have time. This will likely take some time.

@jessLryan
Copy link
Contributor Author

@FabianHofmann I've come across a bug: we need to change the method so that buses are added first (otherwise new components e.g. lines, which are attached to the new buses will fail to be added due to a check at Line 914 of io.py). I'll update and commit

jessLryan and others added 4 commits December 3, 2023 19:14
need to add buses before other components due to existing check in import_components_from_dataframe method when adding components with "bus" in component dataframe columns (components must not be associated with a bus which is not already present in the network)
@jessLryan
Copy link
Contributor Author

@FabianHofmann I've come across a bug: we need to change the method so that buses are added first (otherwise new components e.g. lines, which are attached to the new buses will fail to be added due to a check at Line 914 of io.py). I'll update and commit

now fixed

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.

Idea? - implement __add__ for networks
2 participants