-
Notifications
You must be signed in to change notification settings - Fork 508
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_outlisthelper - 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_freeFormmixes camelCase with snake_case. Consider renaming it toread_outlist_free_formto 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_freeFormfunction. Adding unit tests to cover varied outlist formats (quoted, unquoted, blank lines) would help prevent regressions.
def read_outlist_freeForm(self,f,module):
| Inputs: f - file handle | ||
| module - of OpenFAST, e.g. SubDyn, SeaState (these modules use this) | ||
| ''' | ||
| all_channels = [] |
Copilot
AI
May 22, 2025
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.
|
Reverted the styles |
Feature or improvement description
This PR streamlines how
openfast_ioreads 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_iotests pass locally.r-test updated to enumerate the various outlets formats: https://github.com/mayankchetan/r-test/tree/ofio_outlist