Slight cleanup on axial expansion changer #1032
Conversation
- still working out downstream effects
|
Coverage drop seems to be coverage spray. Both files changed in this PR did not have any change in coverage. |
|
@albeanth, let's chat offline but id like to know the enabler for being able to get rid of needing to reference the BOL height for thermal expansion. It makes sense to me, but maybe it's not super clear in the description of the PR why this is |
| The "thermal" parameter plays a role as thermal expansion is relative to the | ||
| BOL heights where non-thermal is relative to the most recent height. | ||
| """ | ||
| def axiallyExpandAssembly(self): |
There was a problem hiding this comment.
this could use a more detailed doc string... its rather sparse now. How would i get it to do cold height -> hot height expansion vs a simple updated temperature? Referencing methods to be called before it would help @albeanth
There was a problem hiding this comment.
reading the commit message, perhaps it should be renamed to
axiallyExpandAssemblyHotDims or something like that if thats more of what it does now.
There was a problem hiding this comment.
@albeanth Whoops, perhaps I merged too soon.
Description
Cleaning out unnecessary and confusing functionality.
thermal: bool = FalsefromaxiallyExpandAssembly. It is no longer needed and is not used in thermal expansion applications. To avoid confusion on its use, it is removed here.Also resolving several pylint messages.
Existing unit tests did not need updating to reflect this change/cleanup.
Still working out downstream effects and affected projects.
Checklist
doc/release/0.X.rst) are up-to-date with any bug fixes or new features.docfolder.setup.py.