chore: remove pydantic validation from dto classes #740
Conversation
src/libecalc/domain/infrastructure/energy_components/asset/asset.py
Outdated
Show resolved
Hide resolved
src/libecalc/domain/infrastructure/energy_components/base/component_dto.py
Outdated
Show resolved
Hide resolved
src/libecalc/domain/infrastructure/energy_components/base/component_dto.py
Outdated
Show resolved
Hide resolved
src/libecalc/domain/infrastructure/energy_components/base/component_dto.py
Outdated
Show resolved
Hide resolved
| @@ -114,7 +135,7 @@ def to_dto( | |||
| regularity=regularity, | |||
| consumes=consumes, | |||
| component_conditions=component_conditions, | |||
| stream_conditions_priorities=TypeAdapter(YamlPriorities).dump_python( | |||
| stream_conditions_priorities=self.convert_yaml_priorities( | |||
There was a problem hiding this comment.
We should probably map this similar to v1/the other components, in the mapper (removing to_dto). But we'll see what we end up doing with v2, seems unnecessary to maintain if we have no plan to make users migrate.
frodehk
force-pushed
the
remove-pydantic-in-dto
branch
from
8662822 to
268c135
Compare
frodehk
force-pushed
the
remove-pydantic-in-dto
branch
from
562b01a to
f6a0b55
Compare
| @classmethod | ||
| def check_fuel(cls, fuel): | ||
| def validate_fuel_exist(cls, name: str, fuel: Optional[dict[Period, FuelType]], consumes: ConsumptionType): |
There was a problem hiding this comment.
Is this needed? _resolve_fuel in ConsumerMapper (component_mapper) in from_yaml_to_dto is already checking references. It will raise a DataValidationError if references are wrong.
Update/edit: Yes, it may be needed if fuel reference name is blank/empty string.
| @@ -93,7 +103,7 @@ def get_fuel_consumer( | |||
|
|
|||
| class TestFuelConsumer: | |||
| def test_missing_fuel(self): | |||
There was a problem hiding this comment.
This test is checking for a missing fuel, combined with a non-blank fuel reference. This is actually captured earlier when references are checked in the component mapper (ref comment related to validate_missing_fuel). Do we need it? I have added a new test that may be more relevant: blank fuel reference and missing fuel - this will not be catched when checking for references.
There was a problem hiding this comment.
This can be removed then. Do we have a test for missing fuel where it is actually captured?
There was a problem hiding this comment.
Working on it. Replacing this test.
| assert result.suction_pressures[1].tolist() == [0, 31, 22, 13, 4] | ||
| assert result.discharge_pressures[0].tolist() == [0, 31, 22, 13, 4] | ||
| assert result.discharge_pressures[1].tolist() == [0, 31, 22, 13, 4] | ||
| assert np.array(result.rates[0]).tolist() == [0, 31, 22, 13, 4] |
There was a problem hiding this comment.
What's the point of this? create array then convert to list again?
There was a problem hiding this comment.
No point. Not sure why this has happened. Probably traces after some strange debugging.
| @@ -180,7 +180,7 @@ def test_interpolation_functions(): | |||
| assert not (np.asarray(chart_curve.rate_head_and_efficiency_at_minimum_rate) - [336.0, 1778.7, 0.4717]).any() | |||
| assert not (np.asarray(chart_curve.rate_head_and_efficiency_at_maximum_rate) - [1028, 1460.6, 0.7193]).any() | |||
| assert chart_curve.efficiency_as_function_of_rate(708) == 0.6683 | |||
| assert chart_curve.efficiency_as_function_of_rate(456.5) == 0.546 | |||
| assert np.round(chart_curve.efficiency_as_function_of_rate(456.5), 3) == 0.546 | |||
There was a problem hiding this comment.
Did chart have rounding built in when it was pydantic?
There was a problem hiding this comment.
Chart is still in core, not moved to infrastructure. As far as I understand it has not been pydantic.
| @@ -93,7 +103,7 @@ def get_fuel_consumer( | |||
|
|
|||
| class TestFuelConsumer: | |||
| def test_missing_fuel(self): | |||
There was a problem hiding this comment.
This can be removed then. Do we have a test for missing fuel where it is actually captured?
equinor/ecalc-internal#291
Why is this pull request needed?
Pydantic validation is taken care of in the Yaml classes now. It is not needed in the dto classes, here we need more flexibility.
What does this pull request change?
Remove inheritance from EcalcBaseModel (Pydantic) in Component base class. Remove: field_validators, Field definitions, ConfigDict import and usage. Remove Pydantic specific imports and replace with standard Python types if necessary. In libecalc/domain/infracstructure/energy_components/ this is done for:
In addition the following changes are done:
ComponentValidationExceptionis catched in YamlModel.dto.