Skip to content

Conversation

@eantonini
Copy link

@eantonini eantonini commented Oct 30, 2025

Closes #362 .

Changes proposed in this Pull Request

This PR implements a refreshed convert.py with hopefully clearer arguments and logic. Tests and Jupyter notebooks have been updated to reflect these changes.

  • Newly introduced arguments are:

    • mean_over_time in place of capacity_factor
    • sum_over_time
    • capacity_units
  • Arguments that change meaning are:

    • capacity_factor in place of per_unit
  • Removed arguments are:

    • capacity_factor_timeseries
    • per_unit
  • A new logic has also been introduced to account for cases where aggregation is to be performed (aggregate=True when matrix, shapes or layout is passed) and cases where the capacity and capacity factor have to be computed after the aggregation (capacity_factor=True or return_capacity=True).

  • convert_temperature, convert_soil_temperature, and convert_dewpoint_temperature have been combined into a single convert_temperature because all those functions were simply converting Kelvin to Celsius. They don't seem to be used outside convert.py.

  • Lastly, units are now specified in each of the convert_* function.

Checklist

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

@euronion
Copy link
Collaborator

I'll try to have a look next week. Some brief comments:

@eantonini
Copy link
Author

eantonini commented Nov 10, 2025

Hey @euronion, thanks for your feedback and suggestions.

Since your last comment, I added type hints and checked any overlaps with #346 . Both PRs implement a clearer logic for aggregation. From that PR, I copied the case in which a default capacity layout (1MW per grid cell) is used if no layout or matrix is provided. That PR also adds a feature to resample the output time series. Let me know if that is a feature that you also want to be implemented.

All tests now run error free.

@eantonini eantonini marked this pull request as ready for review November 19, 2025 10:01
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.

CSP has capacity factors > 1 when passing shapes

2 participants