fix(correlation): keep empty diagonals when building z_critical (#320)

[abdulsamie10]

Summary of Changes

Fixes the ValueError from #320: valuation_correlation(total=False).z_critical crashed
on any triangle whose earliest diagonals are entirely missing (e.g. load_sample('xyz'),
the Friedland XYZ insurer data from the issue).

self.probs always has one column per link-ratio diagonal, but the z_critical frame it
is written into was built with .dropna(), which also drops interior all-NaN diagonals.
On triangles with missing leading diagonals the frame ended up with fewer ddims than value
columns, and repr (or anything else that walks ddims) raised
ValueError: Shape of passed values is (1, 10), indices imply (1, 9).

The fix builds the frame by valuation slicing instead, so empty diagonals keep their
column and stay aligned with probs:

• Complete triangles produce identical output to before (checked ddims/odims/values
  against the old construction on raa, clrd, quarterly and genins).
• Incomplete triangles now return a well-formed boolean triangle; diagonals with no data
  test non-significant (False). If you'd rather surface those as NaN it would need a dtype
  change away from bool — happy to discuss, this PR keeps the minimal behavior.
• Removed the stale commented-out construction above these lines.

Because the Ruff workflow lints changed files with per-file-ignores cleared, this also
fixes the pre-existing E741 in DevelopmentCorrelation (I → num_dev_periods, pure
rename), removes the corresponding per-file-ignore from pyproject.toml, and applies
ruff format (0.16.1) to the two touched files.

Added test_val_corr_incomplete_triangle using the existing xyz fixture, which runs on
both the numpy and sparse backends. It fails on main and passes with this change.

Related GitHub Issue(s)

Fixes #320

Additional Context for Reviewers

The one behavioral question worth a reviewer's eye: for diagonals with no data,
z_critical now reports False (not significant) rather than being silently dropped from
the output. That matches the "partial results" option discussed in the issue, and keeps
the column count consistent with z, z_expectation and z_variance.

Local runs: uv run pytest → 1129 passed, 8 skipped. ruff check (with per-file-ignores
cleared) and ruff format --check are clean on the changed files with ruff 0.16.1.

[Copilot]

🟢 Approval recommended

The fix directly addresses the reported shape-mismatch failure mode and includes a targeted regression test to prevent recurrence.

Pull request overview

This PR fixes ValuationCorrelation(..., total=False).z_critical failing on incomplete triangles whose earliest diagonals are entirely missing (issue #320) by constructing the z_critical triangle in a way that preserves all-NaN diagonals so its ddims stay aligned with self.probs.

Changes:

• Rebuild z_critical via valuation slicing (instead of dropna) to keep empty diagonals and avoid shape mismatches on repr/display.
• Clean up Ruff linting by renaming an ambiguous variable (I → num_dev_periods) and removing the now-unneeded per-file ignore.
• Add a regression test covering incomplete triangles (including numpy + sparse backends via the xyz fixture).

File summaries

File	Description
pyproject.toml	Removes the Ruff per-file ignore for E741 now that the underlying ambiguity is fixed.
chainladder/core/tests/test_correlation.py	Adds a regression test ensuring z_critical remains well-formed and printable for incomplete triangles (#320).
chainladder/core/correlation.py	Preserves empty diagonals when building z_critical (fixing the ValueError) and renames I to a non-ambiguous identifier.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[henrydingliu]

@abdulsamie10 welcome! thanks for picking up this old bug. looks to be a single line fix?

curious how you came upon our repo and what your interest in actuarial science is

[github-actions]

Pyright Type Completeness

View the full pyright --verifytypes output for this commit

Project (full chainladder package, at this PR's head): 15.2% of exported symbols fully typed (209 / 1378)

	Known	Ambiguous	Unknown	Total
Project (head)	209	111	1058	1378

Other symbols referenced but not exported by chainladder: 13

	Known	Ambiguous	Unknown	Total
Other (head)	3	1	9	13

Symbols without documentation:

• Functions without docstring: 326
• Functions without default param: 0
• Classes without docstring: 10

Patch (exported symbols added or changed by this PR): 0.0% fully typed (0 / 1)

	Known	Ambiguous	Unknown	Total
Patch	0	0	1	1

Patch symbol details

Symbol	Status	Change
chainladder.core.tests.test_correlation.test_val_corr_incomplete_triangle	❌ unknown	new

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.70%. Comparing base (5f34bcd) to head (22c01e5).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1262   +/-   ##
=======================================
  Coverage   91.70%   91.70%           
=======================================
  Files          93       93           
  Lines        5435     5435           
  Branches      699      699           
=======================================
  Hits         4984     4984           
  Misses        327      327           
  Partials      124      124           

Flag	Coverage Δ
unittests	91.70% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.