chore: move expression evaluator to components #776
Conversation
There was a problem hiding this comment.
Looks good. I think we should make some changes to make sure expression_evaluator and pydantic isn't something we have to deal with in domain/application.
I think this change is a good idea as long as expression evaluator isn't meant to be changed. I don't like the idea of having a setter for expression_evaluator, instead I think we should create a new consumer with another expression_evaluator if that is needed.
The domain classes will then be a "locked" representation of the yaml model. The yaml model describes a single expression_evaluator, so the need to change it in the domain classes seems unnecessary to me. (Probably useful for testing, but we should just use a factory instead).
src/libecalc/presentation/yaml/yaml_types/emitters/yaml_venting_emitter.py
Outdated
Show resolved
Hide resolved
I noticed this. Not entirely sure how it works, but is it always the same instance that is being used everywhere? In that case we can create a singleton, that is accessible everywhere, instead of injecting it explicitly. To e.g consider it as a service; "ExpressionEvaluatorService"? |
| def set_consumer_results(self, consumer_results: dict[str, EcalcModelResult]): | ||
| self.consumer_results = consumer_results |
There was a problem hiding this comment.
Remove set_consumer_result?
| if self.volume.rate.type == RateType.CALENDAR_DAY: | ||
| oil_rates = Rates.to_stream_day( | ||
| calendar_day_rates=np.asarray(oil_rates), regularity=self.regularity_evaluated | ||
| ).tolist() |
| energy_context: ComponentEnergyContext, | ||
| energy_model: EnergyModel, | ||
| ) -> Optional[dict[str, EmissionResult]]: | ||
| self._regularity = energy_model.get_regularity(self.id) |
There was a problem hiding this comment.
Pass regularity into init instead?
If I remember correctly get_regularity for EcalcModel was kind of a workaround since we didn't have the flexibility in these classes as we have now. I don't think we have to pass energy_model into evaluate_emissions any more.
Why is this pull request needed?
Make components able to evaluate expressions. It is a part of the preparation for refactoring of e.g. results.
What does this pull request change?
Sending expression evaluator to all components.