Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Allow retrieving only warm water profile #74

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from

Conversation

jnettels
Copy link
Contributor

@jnettels jnettels commented Feb 3, 2025

I added a new argument to HeatBuilding():

ww_only : boolean
if true, only warm water load is included in the heat load profile

This causes only the warm water profile to be returned, by setting the sigmoid parameters a, b and c to 0 and only keeping parameter d.

Fixes #73
Fixes #59

@jnettels jnettels self-assigned this Feb 3, 2025
@jnettels
Copy link
Contributor Author

jnettels commented Feb 4, 2025

The 'Black' code style error in tools.py is unrelated to my PRs, but I nevertheless fixed it in #72. I need to look into how to fix the coverage issue...

@jnettels
Copy link
Contributor Author

jnettels commented Feb 4, 2025

The new test was 99% written by GitHub Copilot. But the style seems to fit the other tests and it fixed the coverall issue. However, it could be argued that I put the new test class in the wrong file. At this point I need direction from a reviewer.

Once #72 is merged, all checks will pass.

@jnettels jnettels requested a review from p-snft February 4, 2025 08:20
Comment on lines 277 to 279
class TestHeatBuilding:
@classmethod
def setup_class(cls):
Copy link
Member

Choose a reason for hiding this comment

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

Working with a test class does only make sense if there are more than one test functions using that class.

@p-snft
Copy link
Member

p-snft commented Feb 17, 2025

If you didn't state the test was AI generated, I'd have guessed it was copied from some StackOverflow that explains test classes. Maybe, just convert it to a plain test function. (Re-factoring everything to use just some test classes will probably be too much for this PR.)

@jnettels
Copy link
Contributor Author

Thanks for accepting the other PR. Now all checks have passed.
I also changed the test to a function, please let me know if this is what you meant.

@jnettels jnettels marked this pull request as ready for review February 19, 2025 10:15
@jnettels jnettels mentioned this pull request Feb 19, 2025
3 tasks
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BDEW heat profiles are missing an option for warm water only getting ww_incl values when set true
2 participants