-
Notifications
You must be signed in to change notification settings - Fork 491
updated outlist reading in openfast_io #2828
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this 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 streamlines and generalizes how openfast_io
reads module outlists by replacing repetitive parsing blocks with a unified read_outlist
function and version-bumping the package.
- Bumps version from 4.0.4 to 4.0.5 in
pyproject.toml
- Removes hard-coded parsing in multiple readers and calls the new
read_outlist
helper - Updates the submodule pointer for regression tests
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
reg_tests/r-test | Updated the submodule commit for r-test |
openfast_io/pyproject.toml | Bumped package version and removed Python 3.9 classifier |
openfast_io/openfast_io/FAST_reader.py | Consolidated outlist parsing into read_outlist and added read_outlist_freeForm |
Comments suppressed due to low confidence (2)
openfast_io/openfast_io/FAST_reader.py:244
- [nitpick] The method name
read_outlist_freeForm
mixes camelCase with snake_case. Consider renaming it toread_outlist_free_form
to follow Python’s snake_case convention.
def read_outlist_freeForm(self,f,module):
openfast_io/openfast_io/FAST_reader.py:244
- There are no tests targeting the new
read_outlist_freeForm
function. Adding unit tests to cover varied outlist formats (quoted, unquoted, blank lines) would help prevent regressions.
def read_outlist_freeForm(self,f,module):
''' | ||
Replacement for set_outlist that doesn't care about whether the channel is in the outlist vartree | ||
Easier, but riskier because OpenFAST can crash | ||
|
||
Inputs: f - file handle | ||
module - of OpenFAST, e.g. SubDyn, SeaState (these modules use this) | ||
''' | ||
all_channels = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The parsing logic in read_outlist_freeForm
largely duplicates the code in read_outlist
. Consider extracting the common logic into a shared helper to reduce duplication and improve maintainability.
Copilot uses AI. Check for mistakes.
Reverted the styles |
Feature or improvement description
This PR streamlines how
openfast_io
reads the outlist's across modules. Additionally, allowing to read the various outlist formats that OpenFAST allows.Related issue, if one exists
None
Impacted areas of the software
openfast_io
Additional supporting information
Test results, if applicable
openfast_io
tests pass locally.r-test updated to enumerate the various outlets formats: https://github.com/mayankchetan/r-test/tree/ofio_outlist