Skip to content

Conversation

@fangliu117
Copy link
Collaborator

@fangliu117 fangliu117 commented Jul 16, 2025

…ests
Ripley‑L template extracted from the NIDAP template, minus Foundry calls, A clean, importable template under spac.templates, Working unit tests:

  1. template_utils.py – two tiny helpers:
  • load_pickle → reads any pickle file (used for AnnData input).
  • save_pickle → saves object and auto‑creates parent dirs.
  1. ripley_l_template.py
  • ._parse_params loads JSON from string, dict, or file.
  • _validate enforces required keys and fills defaults.
  • run_from_json loads AnnData → validates → calls spac.spatial_analysis.ripley_l with real parameters copied from the original template  → stores the DataFrame under adata.uns['ripley_l_results'] → pickles output.
  • main block allows run the template from shell for ad‑hoc checks.
  1. test_ripley_l_template.py –
  • Creates a temp dir, writes the mock dataset, builds a params dict.
  • Tests direct‑dict and JSON‑file execution.
  • Asserts results are attached and output file exists.
  • Cleans up automatically.
  1. examples/ripley_l_params.json – ready‑made parameter file for manual runs or demos.

notes:

  1. initial commit [eaf0e16]: CI is failing only because GitHub Actions can’t find the package under src/, so import spac breaks in the two new test‑files. Nothing is wrong with the code—just the workflow’s environment.
  2. [3aa787d] tried to fix by installing the project editable inside the workflow, edit .github/workflows/gitflow-py-action.yml -> no help (The upstream action (fnlcr‑dmap/gitflow‑py/.github/workflows/parser.yml) starts its own container and
    overrides environment vars, so the PYTHONPATH set in the caller file never reaches the Python process that runs pytest.)
  3. final commit [fb7343e] refactored the unit tests file by combining mock data and test into one -> unblock CI immediately, no need to fight the reusable workflow.

Copilot AI review requested due to automatic review settings July 16, 2025 02:46
Copy link

Copilot AI left a 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 introduces a new Ripley-L template for spatial analysis, providing a clean, platform-agnostic interface for running Ripley-L analysis with configurable parameters. The template is extracted from the NIDAP template and includes comprehensive testing infrastructure.

Key changes:

  • New template system with parameter validation and flexible input handling (JSON file, string, or dict)
  • Utility functions for pickle file operations with automatic directory creation
  • Complete test suite with synthetic data fixtures for CI/CD integration

Reviewed Changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/spac/templates/ripley_l_template.py Main template implementation with parameter parsing, validation, and Ripley-L analysis execution
src/spac/templates/template_utils.py Utility functions for pickle file loading and saving operations
src/spac/templates/init.py Package initialization with template documentation
tests/templates/test_ripley_l_template.py Unit tests covering dict and JSON file parameter input scenarios
tests/templates/_fixtures.py Mock data generation for testing with synthetic AnnData objects
examples/ripley_l_params.json Example parameter configuration file for template usage
.github/workflows/gitflow-py-action.yml CI configuration update to include src directory in PYTHONPATH

"""Tiny AnnData with coords + phenotype labels for fast tests."""
rng = np.random.default_rng(0)
obs = pd.DataFrame(
{"renamed_phenotypes": np.where(rng.random(n_cells) > 0.5,
Copy link

Copilot AI Jul 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The magic number 0.5 should be extracted as a named constant to improve code clarity and maintainability.

Copilot uses AI. Check for mistakes.
)
X = rng.normal(size=(n_cells, 3))
adata = ad.AnnData(X=X, obs=obs)
adata.obsm["spatial"] = rng.random((n_cells, 2)) * 300
Copy link

Copilot AI Jul 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The magic number 300 should be extracted as a named constant to improve code clarity and maintainability.

Suggested change
adata.obsm["spatial"] = rng.random((n_cells, 2)) * 300
adata.obsm["spatial"] = rng.random((n_cells, 2)) * SPATIAL_SCALING_FACTOR

Copilot uses AI. Check for mistakes.
if isinstance(src, dict):
return src
if isinstance(src, (str, Path)):
text = Path(src).read_text() if str(src).endswith(".json") else src
Copy link

Copilot AI Jul 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The file extension check using string operations is fragile. Consider using Path.suffix for more robust file extension detection.

Suggested change
text = Path(src).read_text() if str(src).endswith(".json") else src
text = Path(src).read_text() if Path(src).suffix.lower() == ".json" else src

Copilot uses AI. Check for mistakes.
@fangliu117 fangliu117 requested a review from ruiheesi July 17, 2025 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants