Skip to content

Added variable descriptions to dataclass variables - #4591

Open
j-a-foster wants to merge 3 commits into
mainfrom
fix-jackf-papercut
Open

Added variable descriptions to dataclass variables#4591
j-a-foster wants to merge 3 commits into
mainfrom
fix-jackf-papercut

Conversation

@j-a-foster

@j-a-foster j-a-foster commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Description

Missing variable desriptions have been added to variables in superconducting.py data classes.

Replaces #4554 as there were rebasing issues.

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.

@j-a-foster
j-a-foster requested a review from a team as a code owner September 10, 2026 10:34
@timothy-nunn timothy-nunn changed the title Fix jackf papercut Added variable descriptions to dataclass variables Sep 10, 2026
@timothy-nunn timothy-nunn self-assigned this Sep 10, 2026
@codecov-commenter

codecov-commenter commented Sep 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.47619% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.05%. Comparing base (add60ca) to head (772fc5e).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
process/models/tfcoil/superconducting.py 90.47% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4591      +/-   ##
==========================================
+ Coverage   49.90%   50.05%   +0.14%     
==========================================
  Files         151      151              
  Lines       29864    29940      +76     
==========================================
+ Hits        14904    14986      +82     
+ Misses      14960    14954       -6     

☔ View full report in Codecov by Harness.
📢 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.

@timothy-nunn timothy-nunn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Just a few style comments

Comment thread process/models/tfcoil/superconducting.py Outdated
Comment thread process/models/tfcoil/superconducting.py Outdated
Comment thread process/models/tfcoil/superconducting.py Outdated


@dataclass(slots=True)
class CICCIntegerTurnGeometry(TFGeneralTurnGeometry):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Not a comment for you Jack, I think this class can inherit from CICCAveragedTurnGeometry which will reduce duplication. Will make an issue.

Comment thread process/models/tfcoil/superconducting.py Outdated
Comment thread process/models/tfcoil/superconducting.py Outdated
Comment on lines +2292 to +2293
"""Cable space area (per turn) [m2]
Includes the area that would be taken up by voids and central cooling channel

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"""Cable space area (per turn) [m2]
Includes the area that would be taken up by voids and central cooling channel
"""Cable space area (per turn), includes the area that would be taken up by voids and central cooling channel [m²]

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Had to split between two lines as it was too long, check you're happy with it.

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.

4 participants