Skip to content

Fix: support Wyscout auto-detection for direct streams - #601

Open
Litju wants to merge 1 commit into
PySport:masterfrom
Litju:fix/wyscout-direct-stream-parse-once
Open

Fix: support Wyscout auto-detection for direct streams#601
Litju wants to merge 1 commit into
PySport:masterfrom
Litju:fix/wyscout-direct-stream-parse-once

Conversation

@Litju

@Litju Litju commented Aug 3, 2026

Copy link
Copy Markdown

Summary

This PR fixes Wyscout automatic data-version detection for caller-provided direct streams.

When data_version was not supplied, the Wyscout provider parsed the complete JSON document to identify V2 or V3. The selected deserializer then attempted to parse the same direct stream again.

For caller-provided streams, the first parse left the stream at EOF, so the second parse failed with a JSON decoding error.

The repaired lifecycle is:

open once
→ parse once
→ detect V2 or V3 from the parsed document
→ deserialize that same parsed document

The public wyscout.load() API is unchanged.

Root cause and repair

The previous automatic path had two independent owners of JSON parsing:

provider:
json.load() for version detection

selected deserializer:
json.load() again for deserialization

This was hidden for some path-backed inputs because Kloppy could materialize a fresh stream for each open. Direct file-like inputs retained the same object and stream position across both operations.

Rewinding would not be an appropriate repair: it would preserve the duplicated full-document parse and would require every direct input to support random access.

The provider already had the complete parsed document, so this PR passes that dictionary forward through WyscoutInputs.

The V2 and V3 deserializers now consume the parsed dictionary instead of reopening or reparsing the raw stream. The existing EventDataDeserializer.deserialize() lifecycle remains unchanged.

Stream ownership

Caller-provided streams remain caller-owned.

Kloppy reads a direct stream to its natural final position but does not close it. Automatic loading no longer requires the stream to implement seek() or tell().

Tests

The permanent regression tests cover:

  • V2 and V3;
  • automatic and explicit version selection;
  • path-backed inputs;
  • direct seekable streams;
  • direct non-seekable streams;
  • caller-owned stream lifetime;
  • semantic equivalence for previously successful paths;
  • filtering, coordinates, metadata, periods, and custom event factories;
  • malformed and unsupported documents.

The public Wyscout behavior matrix changed from:

Before: 8 of 12 cases passed
After:  12 of 12 cases passed

For automatic loading, the document lifecycle changed from:

JSON parses:         2 → 1
logical opens:       2 → 1
full-document reads: 2 → 1

Explicit version loading remains behaviorally equivalent.

Validation

The Wyscout-only change passed the complete Kloppy test suite with frozen dependencies on Python 3.9 through 3.13.

The exact combined state containing this PR and the SkillCorner repair from #599 was also validated across Python 3.9 through 3.13:

462 passed
1 expected xfail
0 failures
0 errors

A final fresh Python 3.13 run of Kloppy's native repository suite reproduced:

462 passed, 1 xfailed

Frozen uv synchronization, repository pre-commit hooks, package build, import smoke, and patch checks also passed.

Combined validation confirmed that the Wyscout and SkillCorner changes preserve each other's provider behavior and introduce no functional, semantic, structural, ownership, or full-suite regression.

Compatibility

The public wyscout.load() API and its supported inputs are unchanged.

WyscoutInputs is an internal serializer data structure. Its event_data field now contains the parsed JSON dictionary instead of a raw byte stream.

Code that directly constructs this internal type with a stream is not preserved. Normal use through wyscout.load() is unaffected.

Scope

This PR changes only:

  • kloppy/_providers/wyscout.py
  • kloppy/infra/serializers/event/wyscout/deserializer_v2.py
  • kloppy/infra/serializers/event/wyscout/deserializer_v3.py
  • kloppy/tests/test_wyscout.py

It does not change generic Kloppy I/O, adapters, compression handling, dependencies, lockfiles, CI configuration, or the public loading API.

Relationship to #469

Issue #469 affects both SkillCorner and Wyscout, but the providers fail through different serialization lifecycles.

SkillCorner is addressed separately in #599. This PR addresses the Wyscout-specific complete-document reparse.

Keeping the repairs separate makes each change provider-local and independently reviewable.

Related to #469.

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