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

Fix capacity factor timeseries in convert.py #346

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

TimFuermann
Copy link

The method convert and aggregate was adopted, so the capacity factor timeseries is returned and all functionality, such as return capacity and per_unit are kept.

Closes # (if applicable).

Change proposed in this Pull Request

Description

Motivation and Context

How Has This Been Tested?

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I tested my contribution locally and it seems to work fine.
  • I locally ran pytest inside the repository and no unexpected problems came up.
  • I have adjusted the docstrings in the code appropriately.
  • I have documented the effects of my code changes in the documentation doc/.
  • I have added newly introduced dependencies to environment.yaml file.
  • I have added a note to release notes doc/release_notes.rst.
  • I have used pre-commit run --all to lint/format/check my contribution

The method convert and aggregate was adopted, so the capacity factor timeseries is returned and all functionality, such as return capacity and per_unit are kept.
@FabianHofmann
Copy link
Contributor

thanks for the PR @TimFuermann, I will try to have a look at it soon

@FabianHofmann
Copy link
Contributor

FabianHofmann commented Jul 11, 2024

Thanks again @TimFuermann, this makes really sense. The function itself is a mess (not your changes!), and it builds one critical core of all our analyses, which is why I postponed the review so long. But it looks good. I would like to have a second pair of eyes looking at it. @lkstrp could you also have a look if the main logics are preserved? It is mainly tracking the action steps within the function being subject to the set of arguments... there is no time pressure on this

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