Add x-axis range control and color customization to Kaplan-Meier plots

[Copilot]

Task: Add x-axis range control and color customization to Kaplan-Meier plots ✅ COMPLETE

Recent Updates (addressing final review feedback)

1. Fixed pylint issues:

   • Changed all indentation to 2 spaces (repository standard)
   • Removed all trailing whitespace
   • Pylint score: 10.00/10 ✅

2. Added Jupytext header to 05_command_line_interface.md:

   • Can now be synced with jupytext --sync like other documentation files
   • Pure Markdown (no Python cells) but compatible with Jupytext workflow
   • Fixed code blocks: Changed from ```bash to ``` to display as markdown examples instead of executable cells

3. Renamed and enhanced test file:

   • Renamed test_cli_documentation.py → test_documentation.py
   • Added test_table_of_contents_sync() - now bidirectional:
     • Verifies all documentation files are listed in TOC
     • Verifies all TOC references point to existing files
   • Bidirectional CLI documentation testing maintained

4. Consolidated and expanded plotting tests:

   • Merged test_xmax.py and test_xmax_edge_cases.py into test_km_plotting.py
   • Added 13 new comprehensive plotting tests covering:
     • Custom title and axis labels (verified via matplotlib API)
     • Custom figure size (verified figure dimensions)
     • Include/exclude nominal curve (verified line presence in plot)
     • Binomial-only and patient-wise-only error bands
     • Legend control (location, no legend)
     • Combining xmax with custom options
     • Different output formats (PDF, PNG)
     • Plot results structure validation
     • Tight layout control
     • xmax with different error band configurations
     • Custom font sizes
   • Enhanced test validation: Tests now verify that options actually take effect, not just that plots are generated
   • Total: 21 plotting tests organized into logical sections

5. Updated .github/copilot-instructions.md:

   • Added "Code Style Guidelines" section at the top
   • Documented 2-space indentation requirement
   • Documented no trailing whitespace requirement
   • Documented to always run pylint after editing
   • Updated documentation structure notes for Jupytext compatibility
   • Updated test file name reference

6. Updated GitHub Actions workflow:

   • Changed test name from "CLI documentation completeness test" to "Documentation tests"
   • Updated module path from test_cli_documentation to test_documentation
   • Added single "KM plotting tests" step for consolidated test file

Implementation Summary

This PR successfully adds x-axis range control to Kaplan-Meier plots through the xmax parameter and color customization through --color arguments.

Changes Made

1. kaplan_meier.py

   • Added get_times_for_plot(xmax) method implementing the time selection algorithm
   • Includes times ≤ xmax and xmax itself for smooth curve termination
   • Maintains backward compatibility via cached times_for_plot property

2. kaplan_meier_likelihood.py

   • Added xmax: float | None to KaplanMeierPlotConfig dataclass
   • Updated plot() to use get_times_for_plot(xmax=config.xmax)
   • Modified _finalize_plot() to set x-axis limits: ax.set_xlim(0, xmax)

3. command_line_interface.py

   • Added --xmax CLI argument
   • Added --color argument for single plots (8 color options)
   • Added --high-color and --low-color arguments for two-group plots
   • Implemented color scheme helper functions
   • Refactored to separate parser creation (_make_kombine_parser(), _make_kombine_twogroups_parser()) from execution
   • Propagated parameters through _extract_common_plot_config_args()

4. Documentation

   • docs/kombine/05_command_line_interface.md - Complete CLI reference with Jupytext header
   • Uses plain code blocks (``` instead of ```bash) so examples display as markdown, not executable cells
   • Updated docs/kombine/01_table_of_contents.md to include new file
   • Updated .github/copilot-instructions.md with code style guidelines and documentation structure

5. Tests

   • test_km_plotting.py - Comprehensive KM plotting tests (21 tests) including:
     • 4 xmax functionality tests
     • 4 xmax edge case tests
     • 13 additional plotting configuration tests
     • Tests now verify options work correctly (e.g., figsize verified, labels checked, line presence validated)
   • test_documentation.py - Bidirectional CLI↔docs testing + bidirectional TOC sync verification
   • All tests integrated into GitHub Actions CI/CD workflow

Validation Results

✅ All Success Criteria Met:

• CLI accepts --xmax parameter without errors in both commands
• CLI accepts color arguments (--color, --high-color, --low-color)
• Kaplan-Meier plots display data only up to xmax
• Curve extends smoothly to xmax
• X-axis range correctly limited to [0, xmax] in matplotlib
• Colors can be customized via CLI with 8 options
• Omitting xmax produces full-range plots (backward compatible)
• No regressions or parameter conflicts

✅ Quality Checks:

• All tests pass (20 existing + 21 KM plotting + 2 documentation + 1 discrete optimization)
• Pylint: 10.00/10 (2-space indentation, no trailing whitespace)
• Pyflakes: No issues
• Enhanced test validation: Configuration options are verified to work correctly via matplotlib API
• Comprehensive test coverage for plotting features
• CLI documentation bidirectionally verified
• TOC bidirectionally synchronized with actual documentation files
• All documentation files compatible with Jupytext
• CLI documentation converts to notebook with markdown-only cells (no executable bash cells)
• Documentation follows repository style

Test Coverage

KM Plotting Tests (21 tests with enhanced validation):

• X-axis range control (xmax): 8 tests
• Plot configuration options: 7 tests (now with actual verification)
• Error band options: 3 tests
• Basic plotting: 3 tests

Key Test Enhancements:

• test_plot_with_custom_title_labels: Verifies title and labels are actually set via ax.get_title(), ax.get_xlabel(), ax.get_ylabel()
• test_plot_with_custom_figsize: Verifies figure size via fig.get_size_inches()
• test_plot_exclude_nominal: Verifies nominal line is not in plot by checking ax.get_lines()
• test_plot_no_legend: Tests legend_loc=None configuration

Usage Examples

CLI with xmax:

kombine datacard.txt output.pdf --xmax 10.0

CLI with custom colors:

kombine datacard.txt output.pdf --color green
kombine_twogroups datacard.txt output.pdf --parameter-threshold 0.45 --high-color purple --low-color orange

Python API:

from kombine.datacard import Datacard

datacard = Datacard.parse_datacard("datacard.txt")
kml = datacard.km_likelihood()
kml.plot(xmax=10.0, saveas="output.pdf")

See docs/kombine/05_command_line_interface.md for complete CLI documentation.

Original prompt

Task: Add x-axis range control to Kaplan–Meier plots

Goal:
Allow limiting the x-axis range of Kaplan–Meier plots.

Steps:

1. Add an xmax parameter to the Kaplan–Meier plotting workflow.
2. When computing times_for_plot, include:
   • All times <= xmax, and
   • The first time value greater than xmax (for interpolation or curve continuity only, not for plotting).
3. Use that extra value to calculate the survival probability at xmax, ensuring the curve ends smoothly at the boundary.
4. Set the matplotlib x-axis range to (0, xmax) using plt.xlim() or equivalent.
5. Propagate the new xmax parameter through all relevant functions.
6. Expose this option in the command-line interface (kombine/command_line_interface.py) as --xmax (or --max-time).
7. Ensure full-range plotting behavior is unchanged when xmax is not provided.

Validation:
Confirm that plots truncate exactly at xmax, with a continuous survival curve that ends smoothly at the limit.

Success Criteria:

• The command-line interface accepts --xmax without errors.
• The Kaplan–Meier plot displays data only up to xmax.
• The final segment of the survival curve extends smoothly to xmax using the next time point for calculation only.
• The x-axis range is correctly limited in matplotlib output.
• Omitting --xmax results in a full-range plot identical to current behavior.
• No regressions or parameter naming conflicts occur in existing code paths.

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[hroskes]

@copilot That was a good idea to include xmax itself in the time point selection. That being the case, we don't need to include the first time > xmax.

[Copilot]

@copilot That was a good idea to include xmax itself in the time point selection. That being the case, we don't need to include the first time > xmax.

Updated in commit e2cc023. The implementation now only includes times ≤ xmax and xmax itself, without the first time > xmax. This simplifies the algorithm while maintaining smooth curve termination at the boundary.

[hroskes]

@copilot Please also add command line arguments to vary the color for a single or two_groups Kaplan-Meier plot. We should give a few options (e.g. blue, red, green, and others) that the user can specify. The best fit curve should be that color, while the confidence level bands should be variations of the color, similar to how it's done now (except that currently that colors are hardcoded).

Please also remove the XMAX_USAGE.md and instead document it more similar to how the other documentation in the repo is done.

Also, please incorporate the new tests into the GitHub Actions workflow.

[Copilot]

@copilot Please also add command line arguments to vary the color for a single or two_groups Kaplan-Meier plot. We should give a few options (e.g. blue, red, green, and others) that the user can spec...

Completed in commit 6821239:

1. Color arguments added:

   • Single plots: --color with options: blue, red, green, purple, orange, teal, brown, pink
   • Two-group plots: --high-color and --low-color with same options
   • Each color has matching shades for confidence bands

2. Documentation updated: Removed XMAX_USAGE.md and integrated xmax/color documentation into docs/kombine/03_kaplan_meier_example.md following the repository's documentation style

3. Tests added to CI/CD: Both test_xmax.py and test_xmax_edge_cases.py are now in .github/workflows/ci-cd.yml

Example usage:

kombine datacard.txt output.pdf --color green
kombine_twogroups datacard.txt output.pdf --parameter-threshold 0.45 --high-color purple --low-color orange

[hroskes]

@copilot

Please revert the changes to 03_kaplan_meier_examples.md. That is a Jupyter notebook run with Jupytext and is not meant to illustrate the command line interface.

Instead, please make a new Markdown file, with no Python cells, illustrating the command line interface. Please document all command line options in that file, organized into logical groups via Markdown headers. Please also implement a unit test that checks that all command line options for KoMbine are documented in that file.

Please also document the documentation structure in .github/copilot-instructions.md.

[Copilot]

@copilot

Please revert the changes to 03_kaplan_meier_examples.md. That is a Jupyter notebook run with Jupytext and is not meant to illustrate the command line interface.
...

Completed in commit 6caae84:

1. Reverted 03_kaplan_meier_example.md: Changes reverted - it remains a Jupyter notebook

2. Created docs/kombine/05_command_line_interface.md: Pure Markdown file (no Python cells) documenting all CLI options organized by logical groups:

   • Common arguments (required, error bands, appearance, legend, fonts, computational)
   • kombine_twogroups specific arguments (group separation, p-value options)
   • Usage examples for both commands

3. Refactored CLI for testability:

   • _make_kombine_parser() - builds parser for kombine command
   • _make_kombine_twogroups_parser() - builds parser for kombine_twogroups command
   • Test now uses actual parsers (no code duplication)

4. Enhanced test_cli_documentation.py:

   • Bidirectional testing: verifies all CLI args are documented AND all documented args exist in CLI
   • Catches missing documentation and typos/outdated docs
   • Added to CI/CD workflow

5. Updated .github/copilot-instructions.md: Documents the documentation structure and explains that 05_command_line_interface.md is pure Markdown for CLI (not Python API)

[hroskes]

@copilot

Please fix pylint. Note that pylint is configured to expect two-space indentation. Also, there is some trailing whitespace. Please document in copilot-instructions.md that indents should be two spaces and to check pylint after editing and fix any errors. Nontrivial errors that would require major refactoring to fix can be ignored via magic pylint comments.

Also, please make sure that 05_command_line_interface.md can be converted via jupytext as the other notebooks can. It won't have pure python, but should still run through Jupytext as the table of contents does.

Also, please test that the table of contents is synced with the actual documentation in the folder. You can put this test in test_cli_documentation.py and rename it to test_documentation.py.

[Copilot]

@copilot

Please fix pylint. Note that pylint is configured to expect two-space indentation. Also, there is some trailing whitespace. Please document in copilot-instructions.md that indents shoul...

Completed in commit 8161b55:

1. Fixed pylint: All indentation changed to 2 spaces, removed trailing whitespace. Score: 10.00/10

2. Added Jupytext header to 05_command_line_interface.md: Now compatible with jupytext --sync workflow

3. Enhanced documentation tests:

   • Renamed test_cli_documentation.py → test_documentation.py
   • Added test_table_of_contents_sync() to verify TOC lists all documentation files
   • Both tests pass ✅

4. Updated .github/copilot-instructions.md:

   • Added "Code Style Guidelines" section documenting:
     • Use 2-space indentation
     • No trailing whitespace
     • Always run pylint after editing and fix errors
     • Nontrivial errors can be ignored via # pylint: disable=error-name comments
   • Updated documentation structure notes

[hroskes]

@copilot
test_table_of_contents_sync should be bidirectional and also catch files that are in the table of contents but don't exist.

[Copilot]

@copilot
test_table_of_contents_sync should be bidirectional and also catch files that are in the table of contents but don't exist.

Updated in commit 11fbaf7. The test_table_of_contents_sync function is now bidirectional:

Direction 1: Verifies all documentation files are listed in the TOC
Direction 2: Verifies all TOC references point to existing files

The test now catches both missing documentation and broken references in the table of contents.

[hroskes]

@copilot

I'm getting errors when converting 05_command_line_interface.md to ipynb and running it. This is because of the bash commands, which are being converted to cells with %%bash prefaced, but datacard.txt does not exist.

I don't want those commands to be run, but I do want the code to be displayed in markdown with code tags. In other words, I want the converted notebook to only have markdown cells, but I do want the command line examples to be there.

[Copilot]

@copilot

I'm getting errors when converting 05_command_line_interface.md to ipynb and running it. This is because of the bash commands, which are being converted to cells with %%bash prefaced...

Fixed in commit 0e624f3. Changed all code blocks in 05_command_line_interface.md from ```bash to plain ``` blocks.

When converted to Jupyter notebook via Jupytext, the file now produces only markdown cells (no executable bash cells). The command-line examples are displayed as formatted code within markdown, exactly as requested.

[hroskes]

@copilot

Please merge test_xmax.py and test_xmax_edge_cases.py into a single file, test_km_plotting.py. Please add other relevant plotting tests to that file as well.