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

Review/897/refactoring #908

Conversation

Flix6x
Copy link
Contributor

@Flix6x Flix6x commented Nov 23, 2023

I split my post-review work into two parts. This one deals with refactoring only.

Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x Flix6x self-assigned this Nov 23, 2023
@Flix6x
Copy link
Contributor Author

Flix6x commented Nov 23, 2023

Also, I feel we should be using dashed - rather than underscored - fields for serialized sensor/asset attributes in our database. This should results in a more consistent experience to API/UI users in the long run. We could of course make that a separate issue, but we are introducing two new fields in #897, so we might as well tackle those there. What do you think @victorgarcia98?

If you agree, let's hold off on refactoring until merging this PR and also after considering my next post-review PR, to save us the merge conflicts.

@Flix6x Flix6x merged commit f39c932 into feature/planning/battery-power-capacity-as-sensor Nov 24, 2023
11 of 14 checks passed
@Flix6x Flix6x deleted the review/897/refactoring branch November 24, 2023 08:54
@victorgarcia98
Copy link
Contributor

victorgarcia98 commented Nov 24, 2023

Also, I feel we should be using dashed - rather than underscored - fields for serialized sensor/asset attributes in our database. This should results in a more consistent experience to API/UI users in the long run. We could of course make that a separate issue, but we are introducing two new fields in #897, so we might as well tackle those there. What do you think @victorgarcia98?

You're right, this is worth its own issue. In any case, we shouldn't mix dashed and underscored in the DB.

Issue #911

victorgarcia98 added a commit that referenced this pull request Dec 19, 2023
* add (device) consumption capacity and production capacity as sensors

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* convert units of the power limits to the sensor units

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add QuantityOrSensor field

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add annotations

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* introduce QuantityorSensor schema

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add from  __future__  import annotations

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* remove discarded revision

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add target unit

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* change exception type to FMValidationError

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add explicity default value

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* split into two functions

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* Review/897/refactoring (#908)

* refactor: fix spelling

Signed-off-by: F.N. Claessen <felix@seita.nl>

* style: missing type annotation

Signed-off-by: F.N. Claessen <felix@seita.nl>

* refactor: method and variable renaming

Signed-off-by: F.N. Claessen <felix@seita.nl>

* refactor: add test case explanations

Signed-off-by: F.N. Claessen <felix@seita.nl>

---------

Signed-off-by: F.N. Claessen <felix@seita.nl>

* Review/897/distinguish fallback value and max value (#909)

* refactor: fix spelling

Signed-off-by: F.N. Claessen <felix@seita.nl>

* style: missing type annotation

Signed-off-by: F.N. Claessen <felix@seita.nl>

* refactor: method and variable renaming

Signed-off-by: F.N. Claessen <felix@seita.nl>

* refactor: add test case explanations

Signed-off-by: F.N. Claessen <felix@seita.nl>

* fix: separate logic for falling back on a default attribute and applying a maximum capacity limit

Signed-off-by: F.N. Claessen <felix@seita.nl>

* fix: fill gaps in capacity sensor data using the max_value rather than with the fallback

Signed-off-by: F.N. Claessen <felix@seita.nl>

* fix: type annotation (see https://docs.python.org/3/library/typing.html#typing.Optional)

Signed-off-by: F.N. Claessen <felix@seita.nl>

* refactor: clearly separate steps of falling back and performing a nanmin

Signed-off-by: F.N. Claessen <felix@seita.nl>

* refactor: simplify and have get_quantity_from_attribute return a Quantity, as its name already suggested

Signed-off-by: F.N. Claessen <felix@seita.nl>

* refactor: switch argument order

Signed-off-by: F.N. Claessen <felix@seita.nl>

* refactor: update docstring

Signed-off-by: F.N. Claessen <felix@seita.nl>

* style: flake8

Signed-off-by: F.N. Claessen <felix@seita.nl>

---------

Signed-off-by: F.N. Claessen <felix@seita.nl>

* fix test

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add changelog entry

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* update changelog

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

---------

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Co-authored-by: Felix Claessen <30658763+Flix6x@users.noreply.github.com>
victorgarcia98 added a commit that referenced this pull request Dec 19, 2023
* add (device) consumption capacity and production capacity as sensors

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* convert units of the power limits to the sensor units

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add QuantityOrSensor field

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add annotations

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* introduce QuantityorSensor schema

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add from  __future__  import annotations

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* remove discarded revision

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add target unit

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* change exception type to FMValidationError

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add explicity default value

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* split into two functions

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add usage forecast

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add test

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* Review/897/refactoring (#908)

* refactor: fix spelling

Signed-off-by: F.N. Claessen <felix@seita.nl>

* style: missing type annotation

Signed-off-by: F.N. Claessen <felix@seita.nl>

* refactor: method and variable renaming

Signed-off-by: F.N. Claessen <felix@seita.nl>

* refactor: add test case explanations

Signed-off-by: F.N. Claessen <felix@seita.nl>

---------

Signed-off-by: F.N. Claessen <felix@seita.nl>

* use resolution to convert energy series to the target event resolution

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* simplify conftest

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* Review/897/distinguish fallback value and max value (#909)

* refactor: fix spelling

Signed-off-by: F.N. Claessen <felix@seita.nl>

* style: missing type annotation

Signed-off-by: F.N. Claessen <felix@seita.nl>

* refactor: method and variable renaming

Signed-off-by: F.N. Claessen <felix@seita.nl>

* refactor: add test case explanations

Signed-off-by: F.N. Claessen <felix@seita.nl>

* fix: separate logic for falling back on a default attribute and applying a maximum capacity limit

Signed-off-by: F.N. Claessen <felix@seita.nl>

* fix: fill gaps in capacity sensor data using the max_value rather than with the fallback

Signed-off-by: F.N. Claessen <felix@seita.nl>

* fix: type annotation (see https://docs.python.org/3/library/typing.html#typing.Optional)

Signed-off-by: F.N. Claessen <felix@seita.nl>

* refactor: clearly separate steps of falling back and performing a nanmin

Signed-off-by: F.N. Claessen <felix@seita.nl>

* refactor: simplify and have get_quantity_from_attribute return a Quantity, as its name already suggested

Signed-off-by: F.N. Claessen <felix@seita.nl>

* refactor: switch argument order

Signed-off-by: F.N. Claessen <felix@seita.nl>

* refactor: update docstring

Signed-off-by: F.N. Claessen <felix@seita.nl>

* style: flake8

Signed-off-by: F.N. Claessen <felix@seita.nl>

---------

Signed-off-by: F.N. Claessen <felix@seita.nl>

* fix test

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add changelog entry

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* improve tests and implement consumption_is_positive

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add resolution factor to convert from energy units to power units

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* fix test + add new case

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* split stock delta into stock gain and stock usage

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* update changelog

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* switch to power units

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* improve comments

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add changelog

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* Update changelog.rst

Fix typo (*los componentes*) and swap out some changelog entries between Features and Infrastructure.

Signed-off-by: Felix Claessen <30658763+Flix6x@users.noreply.github.com>

* Apply black

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

---------

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: Victor <victor@seita.nl>
Signed-off-by: Felix Claessen <30658763+Flix6x@users.noreply.github.com>
Co-authored-by: Felix Claessen <30658763+Flix6x@users.noreply.github.com>
Flix6x added a commit that referenced this pull request Dec 21, 2023
* add (device) consumption capacity and production capacity as sensors

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* convert units of the power limits to the sensor units

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add QuantityOrSensor field

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add annotations

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* introduce QuantityorSensor schema

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add from  __future__  import annotations

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* remove discarded revision

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add target unit

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* change exception type to FMValidationError

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add explicity default value

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* split into two functions

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* Review/897/refactoring (#908)

* refactor: fix spelling

Signed-off-by: F.N. Claessen <felix@seita.nl>

* style: missing type annotation

Signed-off-by: F.N. Claessen <felix@seita.nl>

* refactor: method and variable renaming

Signed-off-by: F.N. Claessen <felix@seita.nl>

* refactor: add test case explanations

Signed-off-by: F.N. Claessen <felix@seita.nl>

---------

Signed-off-by: F.N. Claessen <felix@seita.nl>

* Review/897/distinguish fallback value and max value (#909)

* refactor: fix spelling

Signed-off-by: F.N. Claessen <felix@seita.nl>

* style: missing type annotation

Signed-off-by: F.N. Claessen <felix@seita.nl>

* refactor: method and variable renaming

Signed-off-by: F.N. Claessen <felix@seita.nl>

* refactor: add test case explanations

Signed-off-by: F.N. Claessen <felix@seita.nl>

* fix: separate logic for falling back on a default attribute and applying a maximum capacity limit

Signed-off-by: F.N. Claessen <felix@seita.nl>

* fix: fill gaps in capacity sensor data using the max_value rather than with the fallback

Signed-off-by: F.N. Claessen <felix@seita.nl>

* fix: type annotation (see https://docs.python.org/3/library/typing.html#typing.Optional)

Signed-off-by: F.N. Claessen <felix@seita.nl>

* refactor: clearly separate steps of falling back and performing a nanmin

Signed-off-by: F.N. Claessen <felix@seita.nl>

* refactor: simplify and have get_quantity_from_attribute return a Quantity, as its name already suggested

Signed-off-by: F.N. Claessen <felix@seita.nl>

* refactor: switch argument order

Signed-off-by: F.N. Claessen <felix@seita.nl>

* refactor: update docstring

Signed-off-by: F.N. Claessen <felix@seita.nl>

* style: flake8

Signed-off-by: F.N. Claessen <felix@seita.nl>

---------

Signed-off-by: F.N. Claessen <felix@seita.nl>

* fix test

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add changelog entry

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* update changelog

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add charge and discharge efficiencies

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* fix test

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add extra validation and use 100 for the missing values instead of the roundtrip

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* fix validation rules

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* check roundtrip efficiency

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add documentation

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

---------

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Co-authored-by: Felix Claessen <30658763+Flix6x@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants