Skip to content

Implement Model ABC on all models#4146

Merged
timothy-nunn merged 2 commits intomainfrom
add-Model-to-all-process-models
Mar 30, 2026
Merged

Implement Model ABC on all models#4146
timothy-nunn merged 2 commits intomainfrom
add-Model-to-all-process-models

Conversation

@clmould
Copy link
Copy Markdown
Collaborator

@clmould clmould commented Mar 26, 2026

Description

First step for the new dataclass structure is the update all PROCESS models so they implement the Model abstract base class.

Checklist

I confirm that I have completed the following checks:

  • My changes follow the PROCESS style guide
  • I have justified any large differences in the regression tests caused by this pull request in the comments.
  • I have added new tests where appropriate for the changes I have made.
  • If I have had to change any existing unit or integration tests, I have justified this change in the pull request comments.
  • If I have made documentation changes, I have checked they render correctly.
  • I have added documentation for my change, if appropriate.

@timothy-nunn timothy-nunn changed the title update all process models to implement the Model abstract base class Implement Model ABC on all models Mar 26, 2026
@clmould clmould marked this pull request as ready for review March 26, 2026 14:25
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 26, 2026

Codecov Report

❌ Patch coverage is 49.37500% with 81 lines in your changes missing coverage. Please review.
✅ Project coverage is 47.86%. Comparing base (87727f9) to head (0b7dd5d).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
process/core/output.py 0.00% 23 Missing ⚠️
process/core/caller.py 0.00% 17 Missing ⚠️
process/models/power.py 23.52% 13 Missing ⚠️
process/models/physics/physics.py 50.00% 6 Missing ⚠️
process/models/fw.py 50.00% 3 Missing ⚠️
process/models/tfcoil/base.py 71.42% 2 Missing ⚠️
process/models/tfcoil/superconducting.py 50.00% 2 Missing ⚠️
process/models/availability.py 75.00% 1 Missing ⚠️
process/models/blankets/dcll.py 66.66% 1 Missing ⚠️
process/models/blankets/hcpb.py 66.66% 1 Missing ⚠️
... and 12 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4146      +/-   ##
==========================================
+ Coverage   47.66%   47.86%   +0.19%     
==========================================
  Files         143      143              
  Lines       29988    30051      +63     
==========================================
+ Hits        14294    14384      +90     
+ Misses      15694    15667      -27     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@clmould clmould requested a review from a team as a code owner March 26, 2026 14:25
@clmould clmould mentioned this pull request Mar 26, 2026
40 tasks
Copy link
Copy Markdown
Collaborator

@timothy-nunn timothy-nunn left a comment

Choose a reason for hiding this comment

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

Just some cases where you are doing

def output(self):
  self.other_output_method()

where I think we should just rename other_output_method to output so that we don't have an output method defined just to call one other method.

@clmould clmould force-pushed the add-Model-to-all-process-models branch from dbd8d20 to 0b7dd5d Compare March 30, 2026 08:37
@clmould clmould requested a review from timothy-nunn March 30, 2026 08:54
@timothy-nunn timothy-nunn merged commit 94b8561 into main Mar 30, 2026
10 checks passed
@timothy-nunn timothy-nunn deleted the add-Model-to-all-process-models branch March 30, 2026 13:39
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