Skip to content

fix: compare SkillCorner ball IDs by value - #185

Open
Litju wants to merge 1 commit into
floodlight-sports:developfrom
Litju:fix/skillcorner-ball-id-equality
Open

fix: compare SkillCorner ball IDs by value#185
Litju wants to merge 1 commit into
floodlight-sports:developfrom
Litju:fix/skillcorner-ball-id-equality

Conversation

@Litju

@Litju Litju commented Jul 16, 2026

Copy link
Copy Markdown

Summary

The SkillCorner parser currently associates ball observations using Python object identity:

pID is ball_id

Match metadata and positional tracking data are decoded from separate JSON files. Equal provider identifiers are therefore not guaranteed to refer to the same Python object, even though they represent the same ball.

This can silently leave valid ball coordinates as NaN.

Change

Use identifier value equality for ball association:

pID == ball_id

The production change is limited to this predicate.

Regression protection

Added a compact synthetic regression test that:

  • Writes match metadata and positional observations to independent JSON files.
  • Calls the public read_position_data_json parser.
  • Uses value-equal ball identifiers decoded independently.
  • Verifies the exact expected ball coordinates.
  • Verifies Home and Away player coordinates remain correct.
  • Verifies the ball output shape and framerate.

No proprietary provider data are included.

Measured result

On the deterministic regression fixture:

Metric Before After
Correctly associated ball observations 0/1 1/1
Ball-association recall 0% 100%
Returned ball coordinate [NaN, NaN] [12.5, -3.25]
Valid ball rows left NaN 1 0
Coordinate error [0.0, 0.0]

Home and Away control outputs remain unchanged.

Validation

  • Focused SkillCorner regression: 1 passed
  • SkillCorner test module: 1 passed
  • IO test suite: 9 passed
  • Full test suite: 280 passed
  • CI-equivalent full suite with coverage: 280 passed
  • Pre-commit: passed
  • Black: passed
  • Changed-file flake8: passed with zero findings
  • Import smoke: passed
  • Package build: passed

The full-repository flake8 output retains exactly two pre-existing unrelated E226 findings. This change introduces no new lint findings.

Scope

This PR does not change:

  • SkillCorner schema support
  • Coordinate handling
  • Framerate
  • Possession handling
  • Filtering or interpolation
  • Ball smoothing
  • Other provider integrations
  • Public API structure
  • Dependencies

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.

1 participant