feat: refactor structure of galaxy_tool #404
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements a major refactoring of the galaxy_tool structure, separating concerns into modular Python components. The changes move from a monolithic shell script approach to a more maintainable architecture with dedicated utility functions and configuration files.
Key changes:
- Replaces monolithic shell script with modular Python utilities in
tool_utils.py - Introduces tool-specific configuration files for different SPAC templates
- Simplifies the shell wrapper to focus only on execution orchestration
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| galaxy_tools/tool_utils.py | New comprehensive Python utility module containing all core bridge functionality |
| galaxy_tools/tool_config/setup_analysis_config.py | Configuration file for setup analysis tool parameters |
| galaxy_tools/tool_config/load_csv_files_config.py | Configuration file for CSV loading tool with custom preprocessing |
| galaxy_tools/tool_config/boxplot_config.py | Configuration file for boxplot visualization tool |
| galaxy_tools/run_spac_template.sh | Simplified shell wrapper that delegates to Python utilities |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| (u.startswith('{') and u.endswith('}')): | ||
| try: | ||
| return json.loads(u) | ||
| except: |
There was a problem hiding this comment.
Replace bare except clauses with specific exception handling. Bare except clauses can catch system exits and keyboard interrupts, making debugging difficult.
| except: | |
| except json.JSONDecodeError: |
| df = pd.read_csv(filepath, nrows=1) | ||
| if len(df.columns) > 1 or not df.columns[0].startswith('Unnamed'): | ||
| return df.columns.tolist() | ||
| except: |
There was a problem hiding this comment.
Replace bare except clauses with specific exception handling. Bare except clauses can catch system exits and keyboard interrupts, making debugging difficult.
| except: | |
| except Exception: |
| reader = csv.reader(f, dialect) | ||
| header = next(reader) | ||
| return [h.strip().strip('"') for h in header if h.strip()] | ||
| except: |
There was a problem hiding this comment.
Replace bare except clauses with specific exception handling. Bare except clauses can catch system exits and keyboard interrupts, making debugging difficult.
| except: | |
| except (csv.Error, StopIteration, ValueError): |
| try: | ||
| parsed = json.loads(s) | ||
| params[key] = parsed if isinstance(parsed, list) else [s] | ||
| except: |
There was a problem hiding this comment.
Replace bare except clauses with specific exception handling. Bare except clauses can catch system exits and keyboard interrupts, making debugging difficult.
| except: | |
| except (json.JSONDecodeError, ValueError): |
| result.write_h5ad(output_dirs['analysis']) | ||
| saved_count += 1 | ||
|
|
||
| except ImportError as e: |
There was a problem hiding this comment.
Replace bare except clauses with specific exception handling. Bare except clauses can catch system exits and keyboard interrupts, making debugging difficult.
| return saved_count | ||
|
|
||
| def collect_orphan_files(output_dirs): | ||
| """Move files created in working directory to output folders""" |
There was a problem hiding this comment.
Function docstrings should include parameter descriptions and return value information for better API documentation.
| """Move files created in working directory to output folders""" | |
| """ | |
| Move files created in working directory to output folders. | |
| Args: | |
| output_dirs (dict): A dictionary mapping output types (e.g., 'DataFrames', 'figures') to their respective directory paths. | |
| Returns: | |
| int: The number of files moved to the output folders. | |
| """ |
| return moved_count | ||
|
|
||
| def load_tool_config(template_name): | ||
| """Load tool-specific configuration if exists""" |
There was a problem hiding this comment.
Function docstrings should include parameter descriptions and return value information for better API documentation.
| """Load tool-specific configuration if exists""" | |
| """ | |
| Load tool-specific configuration if it exists. | |
| Args: | |
| template_name (str): The name of the tool template for which to load the configuration. | |
| Returns: | |
| dict: The configuration dictionary for the specified tool template. If no specific configuration | |
| exists, returns a default configuration dictionary. | |
| """ |
| return config | ||
|
|
||
| def load_template_module(template_name): | ||
| """Load the SPAC template module""" |
There was a problem hiding this comment.
Function docstrings should include parameter descriptions and return value information for better API documentation.
| """Load the SPAC template module""" | |
| """ | |
| Load the SPAC template module. | |
| Args: | |
| template_name (str): The name of the template to load. | |
| Returns: | |
| module: The loaded template module. | |
| Raises: | |
| ImportError: If the template module cannot be found. | |
| """ |
| raise ImportError(f"Cannot find template: {template_file}") | ||
|
|
||
| def main(): | ||
| """Main entry point for Galaxy-SPAC bridge""" |
There was a problem hiding this comment.
Function docstrings should include parameter descriptions and return value information for better API documentation.
| """Main entry point for Galaxy-SPAC bridge""" | |
| """ | |
| Main entry point for Galaxy-SPAC bridge. | |
| Parameters: | |
| None | |
| Returns: | |
| None | |
| """ |
| """Configuration for Load CSV Files tool""" | ||
|
|
||
| def preprocess_load_csv(params): | ||
| """Special preprocessing for Load CSV Files""" |
There was a problem hiding this comment.
Function docstring should describe the parameters it accepts and what it returns.
| """Special preprocessing for Load CSV Files""" | |
| """ | |
| Special preprocessing for Load CSV Files. | |
| Args: | |
| params (dict): Dictionary of parameters for the tool, possibly including 'CSV_Files'. | |
| Returns: | |
| dict: The processed parameters dictionary with any necessary modifications. | |
| """ |
No description provided.