add a json output and a plot collection #64
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds JSON metrics export and a plots collection feature to the tabular learner tool. The changes enable structured output of model metrics and diagnostic plots for downstream workflows.
Key changes:
- Added JSON metrics export functionality that captures experiment configuration, model details, and evaluation metrics
- Introduced a plots collection output in the Galaxy tool XML for PNG diagnostic plots
- Refactored plot storage mechanism to use a common
_store_plot()method across regression and classification trainers - Added helper functions for JSON serialization of pandas/numpy data structures
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tools/tabularlearner/tabular_learner.xml | Added metrics_json output and plots collection; updated help documentation; added test assertion for metrics_json |
| tools/tabularlearner/base_model_trainer.py | Implemented JSON serialization helpers, _store_plot method, save_metrics_json method, and refactored setup summary preparation |
| tools/tabularlearner/pycaret_regression.py | Updated to use new _store_plot method instead of direct dictionary assignment |
| tools/tabularlearner/pycaret_classification.py | Updated to use new _store_plot method instead of direct dictionary assignment |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if v is not None: | ||
| frac = float(v) | ||
| elif total_rows and n_train is not None: | ||
| frac = n_train / total_rows if total_rows else 0 |
There was a problem hiding this comment.
Line 358 contains redundant logic. The condition if total_rows else 0 is unnecessary because total_rows is already verified to be truthy on line 357. Simplify to frac = n_train / total_rows.
| frac = n_train / total_rows if total_rows else 0 | |
| frac = n_train / total_rows |
| - **Markdown Report**: A concise Markdown summary suitable for embedding in downstream workflows. | ||
|
|
There was a problem hiding this comment.
The documentation mentions a 'Markdown Report' output but no corresponding markdown output is defined in the section. Either remove this documentation or add the markdown output to the tool definition.
| - **Markdown Report**: A concise Markdown summary suitable for embedding in downstream workflows. |
|
|
||
| # 3) Build setup parameters table | ||
| def encode_image_to_base64(self, img_path: str) -> str: | ||
| with open(img_path, "rb") as img_file: |
There was a problem hiding this comment.
File may not be closed if an exception is raised.
No description provided.