refactor: pump v2 support for single speed #348
Conversation
|
rebased etc. will go through a final check |
| @@ -1211,7 +1216,8 @@ def get_asset_result(self) -> libecalc.dto.result.EcalcModelResult: | |||
| }, | |||
| ) | |||
|
|
|||
| sub_components.append(obj) | |||
| if obj: | |||
| # Temporary v2 only | ||
| # We are not able to currently import PumpModelResult due to circular import, therefore we use duck typing... | ||
| if ( | ||
| len(values) == 1 | ||
| and len(other_values) == 1 | ||
| and hasattr(values[0], "extend") | ||
| and hasattr(other_values[0], "extend") | ||
| ): | ||
| values[0].extend(other_values[0]) |
There was a problem hiding this comment.
Due to the comment above...circular import. Next step with v2 will have to be to sort out a few ofthe ciruclar import issues..
| consumers: List[Union[ElectricityConsumer, ConsumerSystem, Any]] = Field( | ||
| default_factory=list | ||
| ) # Any here is Pump, that cannot be explicitly specified due to circular import ...temporalequipment? | ||
| # Any must be set in order for pydantic to accept/validate the consumer |
There was a problem hiding this comment.
not a quick fix, but solve all the "hacks" where we have incorrect directions of dependencies.
| return pump_yaml.to_temporal_domain_models( | ||
| references=self.__references, timesteps=timevector, fuel=fuel | ||
| ) |
There was a problem hiding this comment.
Can't this be done in energy calculator instead? So we don't have to send timevector here?
There was a problem hiding this comment.
not sure, didnt check, but since we are moving away from energy calculator?
| @@ -33,6 +55,10 @@ class Config: | |||
|
|
|||
| energy_usage_model: YamlTemporalModel[str] | |||
|
|
|||
| # TODO: Currently we share the stream conditions between consumer system and pump. Might change if they deviate ... | |||
| # TODO: We may also want to enforce the names of the streams, and e.g. limit to 1 in -and output stream ...? At least we need to know what is in and out ... | |||
| stream_conditions: Optional[YamlConsumerStreamConditions] | |||
There was a problem hiding this comment.
I still think we should fix the schema here, by not making stream_conditions optional.
| @@ -66,6 +66,9 @@ def test_valid(self): | |||
| ) | |||
| } | |||
|
|
|||
| @pytest.mark.skip( | |||
| reason="Due to circular dependencies we cannot specify a pump as an alternative as a consumer for genset, therefore we must set Any, which makes this not fail." | |||
There was a problem hiding this comment.
every case must be considered separately. Here we must figure out the problem with the pumpcharts I think. We can however not move those to domain yet, as that will cause serialization to not work (unless we add that, but it doesnt make sense to serialize domain objects...). So we must avoid need for serialization of the ecalcmodel first...I think.
Have you remembered and considered?
BREAKING:in footer or!in header if breakingWhy is this pull request needed?
This pull request is needed because of....
What does this pull request change?
Write summary of what this pull request changes if needed.
Issues related to this change: