Skip to content

refactor: remove PumpModelDTO and factory, use PumpModel directly#1066

Merged
frodehk merged 1 commit intomainfrom
refactor/remove-double-mapping-pump-model
Aug 26, 2025
Merged

refactor: remove PumpModelDTO and factory, use PumpModel directly#1066
frodehk merged 1 commit intomainfrom
refactor/remove-double-mapping-pump-model

Conversation

@frodehk
Copy link
Copy Markdown
Contributor

@frodehk frodehk commented Aug 26, 2025

Why is this pull request needed?

This PR simplifies the pump model architecture in the codebase by removing the obsolete PumpModelDTO data class and related factory logic. By eliminating the DTO layer and directly using PumpModel and its subclasses, the code becomes more maintainable and easier to work with, reducing unnecessary complexity and potential sources of error.

What does this pull request change?

  • Removes the PumpModelDTO class and all references to it.
  • Deletes the pump/factory.py file which previously handled DTO-based pump creation.
  • Updates mapping logic and reference services to use PumpModel, PumpSingleSpeed, and PumpVariableSpeed directly.
  • Refactors YAML mappers and associated tests to work with the new structure.

@frodehk frodehk self-assigned this Aug 26, 2025
@frodehk frodehk requested a review from a team as a code owner August 26, 2025 10:41
@frodehk frodehk changed the title refactor: remove double mapping pump model refactor: remove PumpModelDTO and factory, use PumpModel directly Aug 26, 2025
@kjbrak kjbrak requested a review from Copilot August 26, 2025 12:47
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the pump model architecture by removing the obsolete PumpModelDTO layer and directly using PumpModel classes throughout the codebase.

  • Eliminates PumpModelDTO data class and associated factory logic
  • Updates mapper functions to directly instantiate PumpSingleSpeed and PumpVariableSpeed classes
  • Removes the pump/factory.py file and its create_pump_model function

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/libecalc/input/mappers/test_consumer_chart.py Updates test assertions to work with PumpModel instances instead of PumpModelDTO
src/libecalc/presentation/yaml/yaml_entities.py Changes type annotation and isinstance check from PumpModelDTO to PumpModel
src/libecalc/presentation/yaml/mappers/facility_input.py Refactors mapper functions to directly instantiate PumpSingleSpeed/PumpVariableSpeed with proper chart wrapping
src/libecalc/presentation/yaml/mappers/consumer_function_mapper.py Removes factory import and direct create_pump_model calls
src/libecalc/presentation/yaml/domain/reference_service.py Updates type annotations from PumpModelDTO to PumpModel
src/libecalc/domain/process/pump/pump.py Removes PumpModelDTO class definition
src/libecalc/domain/process/pump/factory.py Deletes entire factory file with pump creation logic
Comments suppressed due to low confidence (1)

src/libecalc/presentation/yaml/mappers/facility_input.py:1

  • The arguments to InvalidConsumptionType are reversed. The 'actual' parameter should receive the 'consumes' value, and 'expected' should receive ConsumptionType.ELECTRICITY.
from typing import Union

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@frodehk frodehk merged commit f0f5115 into main Aug 26, 2025
12 checks passed
@frodehk frodehk deleted the refactor/remove-double-mapping-pump-model branch August 26, 2025 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants