-
Notifications
You must be signed in to change notification settings - Fork 6
feat: charting changes to prepare for streamed data #573
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
base: main
Are you sure you want to change the base?
Conversation
707ee02 to
7bf237b
Compare
a4e728b to
3ac97a1
Compare
f4f6b53 to
d3554fe
Compare
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 refactors the charting functionality to prepare for transitioning from CSV-based data sources to a streaming gRPC API. The changes improve separation of concerns, enhance live plotting capabilities, rationalize data structures, and remove the restriction of single-interface plotting.
Changes:
- Refactored data structures to replace component-based indexing with direct component suffixes for better streaming API compatibility
- Enhanced live plotting implementation with improved CSV file incremental reading support
- Added multi-interface plotting capabilities with separate figures per interface
- Introduced protocol definitions and abstraction layers for data sources and readers
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/charts/test_plotdefinition_manager.py | Updated tests to handle multiple interfaces and new data structure with transfer names |
| tests/charts/test_message_dipatcher.py | Updated SeriesData constructor calls to include interface_name parameter |
| tests/charts/test_csv_chartdata.py | Updated assertions to reflect component_suffix field replacing data_index and line_suffixes |
| src/ansys/systemcoupling/core/charts/plotter.py | Split Plotter into FigurePlotter and Plotter classes to support multiple figures per interface |
| src/ansys/systemcoupling/core/charts/plotdefinition_manager.py | Refactored to introduce SubplotManager for per-interface subplot management |
| src/ansys/systemcoupling/core/charts/plot_functions.py | Added protocol definitions and refactored functions to support multiple interfaces with CSV suffix |
| src/ansys/systemcoupling/core/charts/message_dispatcher.py | Replaced if-elif chain with match statement and added logging |
| src/ansys/systemcoupling/core/charts/live_csv_datasource.py | Enhanced to handle multiple CSV files/interfaces with improved logging |
| src/ansys/systemcoupling/core/charts/csv_chartdata.py | Simplified data initialization by removing component-based indexing logic |
| src/ansys/systemcoupling/core/charts/chart_datatypes.py | Updated TransferSeriesInfo and SeriesData to use component_suffix and interface_name |
| src/ansys/systemcoupling/core/adaptor/impl/injected_commands.py | Added solve_with_plot command and enhanced argument handling for multiple interfaces |
| examples/temp_non_sphinx/fluent_fluent.py | Deleted entire example file |
| examples/00-systemcoupling/cht_pipe.py | Added version parameters to MAPDL and Fluent launch calls |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/ansys/systemcoupling/core/adaptor/impl/injected_commands.py
Outdated
Show resolved
Hide resolved
src/ansys/systemcoupling/core/adaptor/impl/injected_commands.py
Outdated
Show resolved
Hide resolved
3f401b0 to
b76a44e
Compare
b76a44e to
31be11f
Compare
The current charts functionality relies on obtaining data from CSV files written by SystemCoupling during the solve. We would like to replace the source of data with a new streaming gRPC API that SystemCoupling will support.
This PR includes a number of changes to help prepare for this:
solve_with_plothas been added.