From 58e35e6d2c5e701475df0a12ed343c1cd0bff2cd Mon Sep 17 00:00:00 2001 From: stxkxs <139715017+stxkxs@users.noreply.github.com> Date: Sat, 19 Sep 2026 09:55:15 -0700 Subject: [PATCH] fix: render the season in progress rather than dropping it at the loader and the window MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two independent defects hid the season under way. Five tools opened on the season before it; the sixth carried its rows but showed none of them. ─── The participation sidecar ─── nfl_data_py's import_pbp_data merges a per-season pbp_participation asset and reads it under the same try as the play-by-play. Its handler names an unbound `Error`, so every failure in that block raises NameError in place of the failure that reached it. nflverse publishes the two assets independently, so a season carrying play-by-play and no sidecar raises NameError, which the loader reads as a season the request cannot obtain and drops whole. _nflverse_pbp declines the merge. No column this repository reads comes from the sidecar: import_pbp_data joins it on play_id and old_game_id and contributes only columns named nowhere in src. The option reaches nfl_data_py bound onto the importer through functools.partial, which leaves _download one keyword — its own timeout — and no way for an importer argument to shadow it. The drop reached further than the red zone view. nflverse publishes a weekly-stats asset per season after its games, so a season in progress reaches the play-by-play derivation in _fetch_weekly_stats, and that path raised the same NameError. Declining the sidecar restores both. ─── What a drop can claim ─── A missing release asset and a failed transfer reach these loaders as one exception, so nothing below them can name a cause. The warning and the RuntimeError name the dataset and the seasons that yielded nothing, and stop there; _SeasonUnavailable says the importer returned no data rather than that the source holds none. The README troubleshooting section draws the same line: a season that returns on the next load was a failed transfer, and one that stays absent is waiting on the source. ─── The weeks that open a season ─── Snap Share and Waiver Wire ended their trend computation on drop_nulls(["rolling_avg", "velocity"]), and velocity needed three rows inside one season, so a season's opening weeks left the frame — and with them the season itself, because both screens build their filters from the trended rows. A share is a measurement and a trend is a comparison. Every week a player has usage for is a row; the week that opens their season carries its share and a null rolling average, delta, velocity and trend. The screens render an absent trend as the em dash they already rendered every other absent measure with, and sort the rows carrying none of it last. Velocity slopes across weeks rather than rows. `shift` counts a player's appearances, so a candidate lag reaches a week whose distance depends on which weeks they were active; each lag is admitted only where the weeks it spans fall inside the window, and the slope divides by those weeks. A player back from an absence longer than the window carries no velocity and no trend: there is no recent series under them. Without that check a returning player's first row divides a whole absence's change by one week and sorts to the top of the alert panel. ─── One line per player-week ─── Weekly stats derived from play-by-play emitted a player twice in a week from two directions. Fumbles grouped on posteam, which on a punt or a kickoff names the kicking team, so a returner's fumble opened a second line under a team they never played for; fumbled_1_team names the fumbler's own. Each role grouped on the player's name beside their id, and nflverse spells a name from the source field of each play, so one id carrying two spellings inside one role split its week into two partial lines. The name is taken off the group instead. Both halves fed the waiver join, where a duplicate became a player trended against their own second row. ─── The season a trade is made in ─── Trade Value gated its season filter on the three completed weeks a ranking rests on, so until a season reached them the filter offered only the season before — presenting a finished season's closing chart as the live one. The filter offers a season as soon as one of its weeks has been played, and the table says what the season is still short of. An empty filter and absent data are separate answers. _no_ranking_message distinguishes a load carrying no played week, a season not yet chosen, a season fewer weeks deep than the floor, and a week not yet chosen. Clearing the season clears the week and team options with it, which otherwise stand offering the weeks of whichever season was chosen before. ─── Tests ─── FakeNflverse records the keyword arguments of every call, and two tests pin the sidecar option on both play-by-play paths. Because that fake replaces the function performing the binding, a third installs a stand-in nfl_data_py module and asserts the importer itself receives the option. The engine tests assert that a season's opening week is kept without a trend, that the week after it slopes across the one week behind it, and that a week beyond the window from the last one played carries neither velocity nor trend. The stats tests cover a fumble on a return and a player spelled two ways inside one role. The view tests cover a season too few weeks deep to rank, a season with no played week, and a cleared season filter. Co-authored-by: stxkxsbot <275011021+stxkxsbot@users.noreply.github.com> --- README.md | 55 +++++++++-------- src/ffb/data/loader.py | 77 +++++++++++++++-------- src/ffb/data/stats.py | 57 +++++++---------- src/ffb/snap_share/engine.py | 55 ++++++++++++----- src/ffb/snap_share/screen.py | 10 ++- src/ffb/trade_value/screen.py | 80 +++++++++++++++++++----- src/ffb/waiver_wire/engine.py | 46 ++++++++++---- src/ffb/waiver_wire/screen.py | 8 ++- tests/conftest.py | 17 ++++++ tests/test_loader.py | 96 ++++++++++++++++++++++++----- tests/test_snap_share.py | 102 ++++++++++++++++++++++++------- tests/test_stats.py | 46 +++++++++++++- tests/test_ui_base.py | 112 ++++++++++++++++++++++++++++++++-- tests/test_waiver_wire.py | 96 ++++++++++++++++++++++------- 14 files changed, 662 insertions(+), 195 deletions(-) diff --git a/README.md b/README.md index 7f7ee76..ffffb9c 100644 --- a/README.md +++ b/README.md @@ -148,32 +148,39 @@ only those opens from disk. Retrying the tool that timed out means pressing **Re which forces every dataset it loads — including the ones already cached — to download again. -**A season nflverse has not published.** The season label rolls over on 1 September, -while nflverse publishes a season's play-by-play, snap counts, weekly stats and injuries -only once its games have been played — so for the opening days of September every tool asks -for a season the source answers `HTTP Error 404` for. That season contributes no rows and -the seasons beside it render: each loader resolves its season list one season at a time, so -an unpublished season costs only itself. - -The skip reaches the interface, because every screen builds its season filter from the -seasons its data carries rows for. The unpublished season is absent from the -filter, and the filter opens on the newest season that has data. Start/Sit and Trade Value -read their season options off the results for the same reason, not off the schedule: -nflverse publishes a schedule months before the season opens, so a schedule alone would -offer a season nothing can be projected or ranked for. - -Nothing negative is cached, so the season joins the filter on the first load after nflverse -publishes it. The price is one failed request per unpublished asset per load. - -**`Failed to load data: nflverse has not published snap counts for season 2026`.** Every -season the tool asked for is unpublished, so the load has nothing to render. The message -names the dataset and the seasons. Loads whose datasets are published are unaffected: a tool -fails only on the dataset it could not resolve at all. +**A season a load does not obtain.** The season label rolls over on 1 September, while +nflverse publishes a season's play-by-play, snap counts, weekly stats and injuries only +once its games have been played — so for the opening days of September every tool asks for +a season the source answers `HTTP Error 404` for. That season contributes no rows and the +seasons beside it render: each loader resolves its season list one season at a time, so a +season that does not resolve costs only itself. + +A transport failure costs the same season the same way, and reaches the loaders as the same +exception. Nothing below the loaders distinguishes the two, so nothing reports which one +happened; a season that disappears on one load and returns on the next was a failed +transfer, and one that stays absent is waiting on the source. + +The drop reaches the interface, because every screen builds its season filter from the +seasons its data carries rows for. The absent season is absent from the filter, and the +filter opens on the newest season that has data. Start/Sit reads its week options off the +results for the same reason, not off the schedule: nflverse publishes a schedule months +before the season opens, so a schedule alone would offer a week nothing can be projected +for. Trade Value offers a season as soon as one of its weeks has been played, which is +earlier than it can rank one — the season being played is the season a trade is made in, +and the chart says what it is still short of rather than falling back to a finished season. + +Nothing negative is cached, so the season joins the filter on the first load that obtains +it. The price is one failed request per absent asset per load. + +**`Failed to load data: No snap counts obtained for season 2026`.** Every season the tool +asked for dropped, so the load has nothing to render. The message names the dataset and the +seasons, and no cause. Loads whose datasets resolve are unaffected: a tool fails only on the +dataset it could not resolve at all. **Any other `Failed to load data:` toast** carries the message the download itself raised. A -season nflverse has not published a weekly-stats asset for is not an error on its own: those -weeks are derived from play-by-play, at the cost of a play-by-play download, and the season -drops out only when neither asset covers it. +season with no weekly-stats asset is not an error on its own: those weeks are derived from +play-by-play, at the cost of a play-by-play download, and the season drops out only when +neither asset covers it. **A table shows a sentence where rows would be.** An empty result renders as a message naming which emptiness it is — a filter combination that matches nothing, a season and week diff --git a/src/ffb/data/loader.py b/src/ffb/data/loader.py index 3dd7e7a..f181be8 100644 --- a/src/ffb/data/loader.py +++ b/src/ffb/data/loader.py @@ -4,11 +4,14 @@ — play-by-play, snap counts, weekly stats, injuries — appear only once games have been played. A season the calendar names can therefore have no asset to read. -Every loader taking a season list resolves it one season at a time, so a season with no -asset contributes no rows while the seasons beside it render. A request whose every -season is unpublished raises rather than yielding an empty frame: a frame with no -columns has no `season` column for a screen to derive its filter options from, and -raises `ColumnNotFoundError` the moment an engine filters it. +Every loader taking a season list resolves it one season at a time, so a season the +request does not obtain contributes no rows while the seasons beside it render. A +request that obtains none of its seasons raises rather than yielding an empty frame: a +frame with no columns has no `season` column for a screen to derive its filter options +from, and raises `ColumnNotFoundError` the moment an engine filters it. + +What a request does not obtain, it cannot explain. A missing asset and a failed +transfer reach these loaders as the same exception, so nothing below names a cause. """ import logging @@ -27,7 +30,7 @@ class _SeasonUnavailable(Exception): - """Signals one season the source holds no asset for.""" + """Signals one season a request did not obtain.""" def _download( @@ -65,43 +68,66 @@ def run() -> None: return outcome["value"] -def _nflverse(importer: str, *args: Any) -> pl.DataFrame: +def _nflverse(importer: str, *args: Any, **options: Any) -> pl.DataFrame: """Fetch a frame from the named nfl_data_py importer, under the download timeout. + `options` are keyword arguments for the importer. Binding them onto the callable + here rather than forwarding them through `_download` leaves that function one + keyword, its own timeout, and no way for an importer argument to shadow it. + nfl_data_py is imported here rather than at module scope because it pulls in pandas and numpy, and the TUI reaches its first frame without paying that cost. """ import nfl_data_py as nfl # type: ignore[import-untyped] - pdf = _download(getattr(nfl, importer), *args) + pdf = _download(partial(getattr(nfl, importer), **options), *args) return pl.from_pandas(pdf) -def _nflverse_season(importer: str, season: int) -> pl.DataFrame: +def _nflverse_season(importer: str, season: int, **options: Any) -> pl.DataFrame: """Fetch one season from the named importer, or raise `_SeasonUnavailable`. - A season with no release asset takes one of two shapes. nfl_data_py reads the asset - URL directly, so a missing asset arrives as the 404 urllib raises: `HTTPError`, - which derives from `OSError`. + A read that does not return a season takes one of two shapes. nfl_data_py reads the + asset URL directly, so a missing asset arrives as the 404 urllib raises: + `HTTPError`, which derives from `OSError`. A transport failure — a reset connection, + a DNS failure — derives from `OSError` too, and arrives the same way. `import_pbp_data` cannot report a failed download at all. Its download handler reads `except Error as e`, and `Error` is bound nowhere in that module, so reaching the handler raises `NameError` in place of the failure that reached it — run `inspect.getsource(nfl.import_pbp_data)` to read the handler. `NameError` is - therefore the only signal that importer emits for a season it cannot download. + therefore the only signal that importer emits for a season it cannot download, + whatever stopped the download. + + Neither shape distinguishes an asset that does not exist from one that could not be + read, which is why nothing here reports why a season dropped. - `TimeoutError` derives from `OSError` too, and a stalled transfer says nothing - about what nflverse holds, so it passes through as itself. + `TimeoutError` derives from `OSError` too, and the caller set that deadline, so it + passes through as itself. Narrowing the catch to this one call into nfl_data_py keeps a `NameError` raised anywhere in this repository a `NameError`. """ try: - return _nflverse(importer, [season]) + return _nflverse(importer, [season], **options) except TimeoutError: raise except (OSError, NameError) as e: - raise _SeasonUnavailable(f"{importer} holds no {season} data: {e}") from e + raise _SeasonUnavailable(f"{importer} returned no {season} data: {e}") from e + + +def _nflverse_pbp(season: int) -> pl.DataFrame: + """One season of play-by-play, without the participation sidecar. + + `import_pbp_data` reads the play-by-play asset and a per-season participation + sidecar under one `try`, and the nflverse releases carry the two independently: + a season can hold play-by-play and no sidecar. Reading both together means the + sidecar's absence discards the play-by-play beside it, so the merge is declined + rather than depended on. No column this repository reads comes from the sidecar — + `import_pbp_data` merges it on `play_id` and `old_game_id` and contributes only + columns named nowhere in `src`. + """ + return _nflverse_season("import_pbp_data", season, include_participation=False) def _cached( @@ -150,23 +176,24 @@ def _by_season( served from a partial the first stored. Raising on a request that resolves nothing keeps an empty frame out of the engines. - The message names the seasons and the source, so the error a screen reports says - which season to wait on. + The message names the seasons it obtained nothing for. What it does not name is a + cause: a season drops on a missing asset and on a failed transfer alike, and the + two are indistinguishable at this layer. """ if not seasons: raise RuntimeError(f"No {label} available: no seasons requested (got {seasons})") frames: list[pl.DataFrame] = [] - unpublished: list[int] = [] + dropped: list[int] = [] for season in sorted(set(seasons)): try: frames.append(_cached(f"{prefix}_{season}", fetch, season, force_refresh=force_refresh)) except _SeasonUnavailable as e: - log.warning("skipping %d, nflverse has published no %s for it: %s", season, label, e) - unpublished.append(season) + log.warning("skipping %d, no %s obtained for it: %s", season, label, e) + dropped.append(season) if not frames: - raise RuntimeError(f"nflverse has not published {label} for {_seasons_phrase(unpublished)}") + raise RuntimeError(f"No {label} obtained for {_seasons_phrase(dropped)}") return pl.concat(frames, how="diagonal_relaxed") @@ -193,7 +220,7 @@ def load_pbp( return _by_season( "play-by-play", "pbp", - partial(_nflverse_season, "import_pbp_data"), + _nflverse_pbp, seasons, force_refresh=force_refresh, ) @@ -229,7 +256,7 @@ def _fetch_weekly_stats(season: int) -> pl.DataFrame: except _SeasonUnavailable as e: log.info("weekly data unavailable for %d, deriving from PBP: %s", season, e) - pbp_df = _nflverse_season("import_pbp_data", season) + pbp_df = _nflverse_pbp(season) roster_df: pl.DataFrame | None try: roster_df = _nflverse_season("import_seasonal_rosters", season) diff --git a/src/ffb/data/stats.py b/src/ffb/data/stats.py index 0fea5e9..74f2d79 100644 --- a/src/ffb/data/stats.py +++ b/src/ffb/data/stats.py @@ -28,29 +28,27 @@ def compute_weekly_stats_from_pbp( plays = pbp.filter(pl.col("play_type").is_in(["pass", "run"])) # ── Receiving stats ────────────────────────────────────── + # The name is taken off the group rather than grouped on. nflverse spells a name + # from the source field of each play, so one id can carry two spellings inside a + # single role in one week; grouping on the spelling would split that week into two + # partial lines. The spelling kept is the one the week's first play carries. rec = ( plays.filter((pl.col("pass_attempt") == 1) & pl.col("receiver_player_id").is_not_null()) .group_by( "receiver_player_id", - "receiver_player_name", "posteam", "season", "week", "season_type", ) .agg( + pl.col("receiver_player_name").first().alias("name_from_receiving"), pl.len().alias("targets"), pl.col("complete_pass").sum().alias("receptions"), pl.col("receiving_yards").sum().alias("receiving_yards"), pl.col("pass_touchdown").sum().alias("receiving_tds"), ) - .rename( - { - "receiver_player_id": "player_id", - "receiver_player_name": "name_from_receiving", - "posteam": "recent_team", - } - ) + .rename({"receiver_player_id": "player_id", "posteam": "recent_team"}) ) # ── Rushing stats ──────────────────────────────────────── @@ -58,24 +56,18 @@ def compute_weekly_stats_from_pbp( plays.filter((pl.col("rush_attempt") == 1) & pl.col("rusher_player_id").is_not_null()) .group_by( "rusher_player_id", - "rusher_player_name", "posteam", "season", "week", "season_type", ) .agg( + pl.col("rusher_player_name").first().alias("name_from_rushing"), pl.len().alias("carries"), pl.col("rushing_yards").sum().alias("rushing_yards"), pl.col("rush_touchdown").sum().alias("rushing_tds"), ) - .rename( - { - "rusher_player_id": "player_id", - "rusher_player_name": "name_from_rushing", - "posteam": "recent_team", - } - ) + .rename({"rusher_player_id": "player_id", "posteam": "recent_team"}) ) # ── Passing stats ──────────────────────────────────────── @@ -83,47 +75,44 @@ def compute_weekly_stats_from_pbp( plays.filter((pl.col("pass_attempt") == 1) & pl.col("passer_player_id").is_not_null()) .group_by( "passer_player_id", - "passer_player_name", "posteam", "season", "week", "season_type", ) .agg( + pl.col("passer_player_name").first().alias("name_from_passing"), pl.col("passing_yards").sum().alias("passing_yards"), pl.col("pass_touchdown").sum().alias("passing_tds"), pl.col("interception").sum().alias("interceptions"), ) - .rename( - { - "passer_player_id": "player_id", - "passer_player_name": "name_from_passing", - "posteam": "recent_team", - } - ) + .rename({"passer_player_id": "player_id", "posteam": "recent_team"}) ) # ── Fumbles (attributed to the fumbler) ────────────────── # Read from the unfiltered frame: a fumble lost on a kickoff or punt return # carries the same -2 as one lost from scrimmage. + # The team is the fumbler's own, which on a return is the team without the ball: + # `posteam` names the kicking team there, and attributing the fumble to it would + # give the returner a second weekly row under a team they never played for. fumbles = ( - pbp.filter((pl.col("fumble_lost") == 1) & pl.col("fumbled_1_player_id").is_not_null()) + pbp.filter( + (pl.col("fumble_lost") == 1) + & pl.col("fumbled_1_player_id").is_not_null() + & pl.col("fumbled_1_team").is_not_null() + ) .group_by( "fumbled_1_player_id", - "fumbled_1_player_name", - "posteam", + "fumbled_1_team", "season", "week", "season_type", ) - .agg(pl.col("fumble_lost").sum().alias("fumbles_lost")) - .rename( - { - "fumbled_1_player_id": "player_id", - "fumbled_1_player_name": "name_from_fumbles", - "posteam": "recent_team", - } + .agg( + pl.col("fumbled_1_player_name").first().alias("name_from_fumbles"), + pl.col("fumble_lost").sum().alias("fumbles_lost"), ) + .rename({"fumbled_1_player_id": "player_id", "fumbled_1_team": "recent_team"}) ) # ── Opponent mapping (one defteam per team per game) ──── diff --git a/src/ffb/snap_share/engine.py b/src/ffb/snap_share/engine.py index 168018c..51ee8be 100644 --- a/src/ffb/snap_share/engine.py +++ b/src/ffb/snap_share/engine.py @@ -15,6 +15,13 @@ def compute_trends( Returns a DataFrame with columns: pfr_player_id, player, position, team, season, week, snap_pct, rolling_avg, delta, velocity, trend. + Every week a player has a snap share for is a row, including the weeks that open + a season. A trend needs earlier weeks to measure against, so the opening week of + a player's season carries a snap share and a null rolling average, delta, + velocity and trend; the weeks after it slope against the earliest week still inside + the window, per week elapsed. A week whose player was absent for longer than the + window carries no velocity and no trend either: there is no recent series under it. + Every rolling window groups by ``pfr_player_id``. Display names collide across the league — multiple active players share a name in any given season — so a name-keyed window splices two players into one series. @@ -50,25 +57,46 @@ def compute_trends( # Delta: current week vs prior rolling average df = df.with_columns((pl.col("snap_pct") - pl.col("rolling_avg")).alias("delta")) - # Velocity: simplified OLS slope = (current - lag(window-1)) / (window-1) + # `shift` counts a player's appearances, not weeks, so a lag reaches a week whose + # distance depends on which weeks the player was active. Each candidate lag is + # admitted only where the weeks it spans fall inside the window, and the slope + # divides by those weeks: a player back from an absence longer than the window has + # no series to slope across, and one back from a shorter absence slopes across the + # weeks that passed rather than the rows. + lags = range(window - 1, 0, -1) + + def earlier(lag: int) -> pl.Expr: + return pl.col("snap_pct").shift(lag).over("pfr_player_id", "season") + + def span(lag: int) -> pl.Expr: + return pl.col("week") - pl.col("week").shift(lag).over("pfr_player_id", "season") + + def inside(lag: int) -> pl.Expr: + return (span(lag) > 0) & (span(lag) <= window - 1) + + # Velocity: simplified OLS slope = (current - earlier) / weeks between them, taken + # across the widest span the window holds. df = df.with_columns( - ( - ( - pl.col("snap_pct") - - pl.col("snap_pct").shift(window - 1).over("pfr_player_id", "season") - ) - / (window - 1) + pl.coalesce( + [ + pl.when(inside(lag)).then((pl.col("snap_pct") - earlier(lag)) / span(lag)) + for lag in lags + ] ).alias("velocity") ) + # The share the window opens on, against which a breakout is measured. + opening = pl.coalesce([pl.when(inside(lag)).then(earlier(lag)) for lag in lags]) + # Breakout: crossed from <50% to >60% within the window - breakout = (pl.col("snap_pct") > 60) & ( - pl.col("snap_pct").shift(window - 1).over("pfr_player_id", "season") < 50 - ) + breakout = (pl.col("snap_pct") > 60) & (opening < 50) - # Classify trend + # Classify trend. A week with nothing behind it has no direction to name, and a + # null says so where any label would assert movement that was never measured. df = df.with_columns( - pl.when(breakout) + pl.when(pl.col("velocity").is_null()) + .then(pl.lit(None, dtype=pl.String)) + .when(breakout) .then(pl.lit("breakout")) .when((pl.col("delta") > delta_threshold) & (pl.col("velocity") > 0)) .then(pl.lit("rising")) @@ -78,5 +106,4 @@ def compute_trends( .alias("trend") ) - # Drop weeks without enough history for trend calculation - return df.drop_nulls(subset=["rolling_avg", "velocity"]) + return df diff --git a/src/ffb/snap_share/screen.py b/src/ffb/snap_share/screen.py index 1d0162a..5012192 100644 --- a/src/ffb/snap_share/screen.py +++ b/src/ffb/snap_share/screen.py @@ -30,7 +30,11 @@ def _trend_row(row: dict[str, Any]) -> tuple[str, ...]: - """Format one trend row as table cells, rendering an absent measure as an em dash.""" + """Format one trend row as table cells, rendering an absent measure as an em dash. + + A season's opening week has no earlier week behind it, so every measure of movement + is absent there and the row carries its snap share alone. + """ snap = row["snap_pct"] avg = row["rolling_avg"] delta = row["delta"] @@ -44,7 +48,7 @@ def _trend_row(row: dict[str, Any]) -> tuple[str, ...]: f"{avg:.1f}%" if avg is not None else "—", f"{delta:+.1f}" if delta is not None else "—", f"{velocity:+.1f}" if velocity is not None else "—", - row["trend"], + row["trend"] or "—", ) @@ -183,7 +187,7 @@ def _update_tables(self, df: pl.DataFrame) -> None: self.fill_table( "sn-all-table", - df.sort("delta", descending=True), + df.sort("delta", descending=True, nulls_last=True), _trend_row, "No snap counts for the selected season, week, position and team.", ) diff --git a/src/ffb/trade_value/screen.py b/src/ffb/trade_value/screen.py index f52f8e5..e98b631 100644 --- a/src/ffb/trade_value/screen.py +++ b/src/ffb/trade_value/screen.py @@ -37,20 +37,26 @@ #: Seasons the load asks for. Production, schedule and usage read the one season #: selected; the health component reads injury weeks across every season loaded, so a #: second season buys a longer availability history. The season filter offers the -#: loaded seasons a ranking can run on, which is narrower: a season nflverse has -#: published a schedule for carries no results until its games have been played. +#: loaded seasons a week has been played in, which is narrower than the schedule: a +#: season carries a schedule months before it carries a result. SEASON_WINDOW = 2 #: Weeks of results a ranking rests on. The engine ranks nobody below this, so the week -#: filter offers no week below it either. +#: filter offers no week below it either. The season filter is not gated on it: a season +#: too few weeks deep to rank is still the season a trade is being made in, and offering +#: the one before it as the default would present a finished season as the live chart. MIN_WEEKS_PLAYED = 3 -#: Stands in for the table when no loaded season carries enough played weeks to rank. -NO_RANKABLE_WEEKS = ( - f"No season has the {MIN_WEEKS_PLAYED} completed weeks a ranking rests on. " - "nflverse publishes a season's weekly stats once its games have been played." +#: Stands in for the table when no loaded season carries a played week at all. +NO_PLAYED_WEEKS = ( + "No loaded season carries a played week. A season's weekly stats appear once its " + "games have been played." ) +#: Stands in for the table where the filters, not the data, leave the chart unranked. +NO_SEASON_SELECTED = "Select a season to rank." +NO_WEEK_SELECTED = "Select a week to rank through." + @dataclass(frozen=True, slots=True) class TradeValueData: @@ -71,13 +77,27 @@ def _rankable_weeks(weekly: pl.DataFrame, season: int) -> list[int]: off the results rather than off the schedule: a schedule carries every week of a season from the moment nflverse publishes it, months before the first is played. """ - played = ( + return sorted( + week + for week in _played_weeks(weekly, season) + if MIN_WEEKS_PLAYED <= week < REGULAR_SEASON_WEEKS + ) + + +def _played_weeks(weekly: pl.DataFrame, season: int) -> list[int]: + """Regular-season weeks of `season` the weekly stats carry results for, in order. + + A season is offered in the filter as soon as one of its weeks has been played, + which is earlier than a ranking can run. The two thresholds are deliberately + apart: what the filter offers is which season the user is looking at, and what + the engine ranks is whether there is enough behind it to rank. + """ + return sorted( weekly.filter((pl.col("season") == season) & (pl.col("season_type") == "REG"))["week"] .unique() .drop_nulls() .to_list() ) - return sorted(week for week in played if MIN_WEEKS_PLAYED <= week < REGULAR_SEASON_WEEKS) def _value_row(row: dict[str, Any]) -> tuple[str, ...]: @@ -152,7 +172,7 @@ def _populate_filters(self) -> None: seasons = sorted( season for season in reg["season"].unique().drop_nulls().to_list() - if _rankable_weeks(data.weekly, season) + if _played_weeks(data.weekly, season) ) season_select = self.query_one("#tv-filter-season", Select) season_select.set_options([(str(s), s) for s in seasons]) @@ -165,12 +185,18 @@ def _update_week_and_team_options(self) -> None: data = self._data if data is None: return + week_select = self.query_one("#tv-filter-week", Select) + team_select = self.query_one("#tv-filter-team", Select) + season = self.selected("tv-filter-season") if season is None: + # A week and a team belong to a season. Left standing, they would offer the + # weeks of whichever season was selected before. + week_select.set_options([]) + team_select.set_options([]) return weeks = _rankable_weeks(data.weekly, season) - week_select = self.query_one("#tv-filter-week", Select) week_select.set_options([(f"Week {w}", w) for w in weeks]) if weeks: week_select.value = weeks[-1] @@ -180,7 +206,6 @@ def _update_week_and_team_options(self) -> None: set(reg["home_team"].unique().drop_nulls().to_list()) | set(reg["away_team"].unique().drop_nulls().to_list()) ) - team_select = self.query_one("#tv-filter-team", Select) team_select.set_options(self.all_options(teams)) team_select.value = ALL @@ -192,11 +217,11 @@ def _compute_values(self) -> None: season = self.selected("tv-filter-season") week = self.selected("tv-filter-week") - # Both filters hold no selection until a load fills their options, and a load - # that reached no rankable week fills neither. + # The season filter holds no selection until a load fills it, and the week + # filter stays empty for a season fewer weeks deep than a ranking rests on. if not isinstance(season, int) or not isinstance(week, int): self._values = None - self.fill_table("tv-table", pl.DataFrame(), _value_row, NO_RANKABLE_WEEKS) + self.fill_table("tv-table", pl.DataFrame(), _value_row, self._no_ranking_message()) return self._values = compute_trade_values( @@ -210,6 +235,31 @@ def _compute_values(self) -> None: ) self._apply_filters() + def _no_ranking_message(self) -> str: + """Say why the chart ranks nobody, separating absent data from an empty filter. + + A season too few weeks deep to rank and a filter the user has cleared leave the + same two filters empty, and each has its own answer: one says what the season + does not yet carry, the other asks for a choice. + """ + data = self._data + if data is None: + return NO_PLAYED_WEEKS + + seasons = data.weekly["season"].unique().drop_nulls().to_list() + if not any(_played_weeks(data.weekly, season) for season in seasons): + return NO_PLAYED_WEEKS + + season = self.selected("tv-filter-season") + if not isinstance(season, int): + return NO_SEASON_SELECTED + if not _rankable_weeks(data.weekly, season): + return ( + f"No trade values for {season}. A ranking rests on {MIN_WEEKS_PLAYED} " + f"completed weeks and {season} carries fewer." + ) + return NO_WEEK_SELECTED + def _apply_filters(self) -> None: values = self._values if values is None: diff --git a/src/ffb/waiver_wire/engine.py b/src/ffb/waiver_wire/engine.py index 4f7ea3c..ecf0e89 100644 --- a/src/ffb/waiver_wire/engine.py +++ b/src/ffb/waiver_wire/engine.py @@ -20,6 +20,13 @@ def compute_usage_trends( Returns: gsis_id, player, position, team, season, week, snap_pct, tgt_share, touch_share, usage_score, rolling_avg, delta, velocity, trend. + Every week a player has usage for is a row, including the weeks that open a + season. A trend needs earlier weeks to measure against, so the opening week of a + player's season carries a usage score and a null rolling average, delta, velocity + and trend; the weeks after it slope against the earliest week still inside the + window, per week elapsed. A week whose player was absent for longer than the window + carries no velocity and no trend either: there is no recent series under it. + Every rolling window groups by ``gsis_id``. Display names collide across the league — multiple active players share a name in any given season — so a name-keyed window splices two players into one series. @@ -107,20 +114,38 @@ def compute_usage_trends( # Delta df = df.with_columns((pl.col("usage_score") - pl.col("rolling_avg")).alias("delta")) - # Velocity (slope) + # `shift` counts a player's appearances, not weeks, so a lag reaches a week whose + # distance depends on which weeks the player was active. Each candidate lag is + # admitted only where the weeks it spans fall inside the window, and the slope + # divides by those weeks: a player back from an absence longer than the window has + # no series to slope across, and one back from a shorter absence slopes across the + # weeks that passed rather than the rows. + lags = range(window - 1, 0, -1) + + def earlier(lag: int) -> pl.Expr: + return pl.col("usage_score").shift(lag).over("gsis_id", "season") + + def span(lag: int) -> pl.Expr: + return pl.col("week") - pl.col("week").shift(lag).over("gsis_id", "season") + + # Velocity: slope per week elapsed, across the widest span the window holds. df = df.with_columns( - ( - ( - pl.col("usage_score") - - pl.col("usage_score").shift(window - 1).over("gsis_id", "season") - ) - / (window - 1) + pl.coalesce( + [ + pl.when((span(lag) > 0) & (span(lag) <= window - 1)).then( + (pl.col("usage_score") - earlier(lag)) / span(lag) + ) + for lag in lags + ] ).alias("velocity") ) - # Trend classification + # Trend classification. A week with nothing behind it has no direction to name, and + # a null says so where any label would assert movement that was never measured. df = df.with_columns( - pl.when((pl.col("delta") > 2) & (pl.col("velocity") > 0)) + pl.when(pl.col("velocity").is_null()) + .then(pl.lit(None, dtype=pl.String)) + .when((pl.col("delta") > 2) & (pl.col("velocity") > 0)) .then(pl.lit("rising")) .when((pl.col("delta") < -2) & (pl.col("velocity") < 0)) .then(pl.lit("falling")) @@ -128,8 +153,7 @@ def compute_usage_trends( .alias("trend") ) - # Drop incomplete rows and select output columns - return df.drop_nulls(subset=["rolling_avg", "velocity"]).select( + return df.select( "gsis_id", "player", "position", diff --git a/src/ffb/waiver_wire/screen.py b/src/ffb/waiver_wire/screen.py index bf4a706..bccb552 100644 --- a/src/ffb/waiver_wire/screen.py +++ b/src/ffb/waiver_wire/screen.py @@ -59,7 +59,7 @@ def _cells(row: dict[str, Any]) -> tuple[str, ...]: _fmt(row["rolling_avg"], ".1f"), _fmt(row["delta"], "+.1f"), _fmt(row["velocity"], "+.1f"), - row["trend"], + row["trend"] or MISSING, ) @@ -182,7 +182,9 @@ def _update_tables(self, df: pl.DataFrame) -> None: """Split the filtered frame into the two alert panels and the full listing. Rising sorts by steepest climb and falling by steepest drop, so the top row of - each panel is the player whose usage moved the most. + each panel is the player whose usage moved the most. The full listing sorts the + same way and puts the weeks carrying no velocity last, behind every week whose + movement is measured. """ rising = df.filter(pl.col("trend") == "rising").sort("velocity", descending=True) self.fill_table( @@ -202,7 +204,7 @@ def _update_tables(self, df: pl.DataFrame) -> None: self.fill_table( "ww-all-table", - df.sort("velocity", descending=True), + df.sort("velocity", descending=True, nulls_last=True), _cells, "No players match these filters. Widen the position, team or week filter.", ) diff --git a/tests/conftest.py b/tests/conftest.py index b570411..8e3ced6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -922,6 +922,7 @@ def breakout_snaps_points_scale(breakout_snaps: pl.DataFrame) -> pl.DataFrame: "rusher_player_name": pl.String, "fumbled_1_player_id": pl.String, "fumbled_1_player_name": pl.String, + "fumbled_1_team": pl.String, } _PBP_ZERO = { @@ -980,9 +981,25 @@ def _carry(team: str, *, yards: float = 0.0, td: float = 0.0, fumbled: float = 0 fields["fumble_lost"] = fumbled fields["fumbled_1_player_id"] = league_gsis_id(team, "RB") fields["fumbled_1_player_name"] = league_player_name(team, "RB") + fields["fumbled_1_team"] = team return fields +def _return_fumble(kicking: str, returning: str) -> dict: + """One punt by `kicking` whose returner, of `returning`, loses the ball. + + `posteam` is the kicking team on a return, so the fumbler's team is the one team + the play does not name as holding the ball. + """ + return { + "play_type": "punt", + "fumble_lost": 1.0, + "fumbled_1_player_id": league_gsis_id(returning, "WR"), + "fumbled_1_player_name": league_player_name(returning, "WR"), + "fumbled_1_team": returning, + } + + def _dead_ball(play_type: str) -> dict: """One snap that is neither a pass nor a rush.""" return {"play_type": play_type} diff --git a/tests/test_loader.py b/tests/test_loader.py index d92a268..9b333a8 100644 --- a/tests/test_loader.py +++ b/tests/test_loader.py @@ -7,7 +7,9 @@ import email.message import json +import sys import threading +import types import urllib.error from collections.abc import Callable from typing import Any @@ -26,21 +28,24 @@ class FakeNflverse: a frame to return, or a callable taking the arguments the loader passes. An importer with no keyword raises `KeyError`, so a loader reaching for data the test did not describe fails loudly. + + A call records its keyword arguments alongside its positional ones, so a test can + assert the options a loader sends an importer and not only the season it asks for. """ def __init__(self, **importers: Any) -> None: self.importers = importers - self.calls: list[tuple[str, tuple[Any, ...]]] = [] + self.calls: list[tuple[str, tuple[Any, ...], dict[str, Any]]] = [] - def __call__(self, importer: str, *args: Any) -> pl.DataFrame: - self.calls.append((importer, args)) + def __call__(self, importer: str, *args: Any, **options: Any) -> pl.DataFrame: + self.calls.append((importer, args, options)) handler = self.importers[importer] return handler(*args) if callable(handler) else handler @property def names(self) -> list[str]: """The importers called, in call order.""" - return [name for name, _ in self.calls] + return [name for name, _, _ in self.calls] def marked(marker: int) -> pl.DataFrame: @@ -125,7 +130,7 @@ def fetch(*args: Any) -> str: def test_a_miss_fetches_from_the_importer_the_loader_names(nflverse): fake = nflverse(import_snap_counts=marked(1)) loader.load_snap_counts([2025]) - assert fake.calls == [("import_snap_counts", ([2025],))] + assert fake.calls == [("import_snap_counts", ([2025],), {})] def test_a_fresh_entry_is_served_without_a_second_fetch(nflverse): @@ -201,7 +206,7 @@ def test_distinct_seasons_of_one_dataset_hold_separate_entries(nflverse): def test_the_player_crosswalk_is_fetched_without_seasons(nflverse): fake = nflverse(import_ids=marked(1)) loader.load_player_ids() - assert fake.calls == [("import_ids", ())] + assert fake.calls == [("import_ids", (), {})] def test_the_player_crosswalk_is_stored_under_a_key_naming_no_seasons(nflverse, cache_dir): @@ -306,7 +311,7 @@ def test_only_the_unpublished_seasons_reach_the_fallback(nflverse, pbp, rosters) ) loader.load_weekly_stats([2024, 2025]) assert [call for call in fake.calls if call[0] == "import_pbp_data"] == [ - ("import_pbp_data", ([2025],)) + ("import_pbp_data", ([2025],), {"include_participation": False}) ] @@ -550,7 +555,7 @@ def test_a_loader_taking_a_season_list_raises_when_the_source_holds_none_of_them nflverse, name, importer ): nflverse(**{importer: source()}) - with pytest.raises(RuntimeError, match="nflverse has not published"): + with pytest.raises(RuntimeError, match="obtained for"): getattr(loader, name)([2026]) @@ -563,8 +568,61 @@ def test_each_season_is_asked_for_in_its_own_request(nflverse): fake = nflverse(import_snap_counts=source(2025, 2026)) loader.load_snap_counts([2026, 2025]) assert fake.calls == [ - ("import_snap_counts", ([2025],)), - ("import_snap_counts", ([2026],)), + ("import_snap_counts", ([2025],), {}), + ("import_snap_counts", ([2026],), {}), + ] + + +def test_the_participation_option_reaches_nfl_data_py(monkeypatch: pytest.MonkeyPatch) -> None: + """The binding is exercised where every other test replaces the function that does it. + + `FakeNflverse` stands in for `loader._nflverse`, so an assertion made through it + observes the arguments the loader passed rather than the arguments nf_data_py + received. Dropping the binding would leave those assertions passing. + """ + import pandas + + received: dict[str, Any] = {} + + def import_pbp_data(seasons: list[int], **options: Any) -> "pandas.DataFrame": + received["seasons"] = seasons + received["options"] = options + return pandas.DataFrame({"season": seasons}) + + module = types.ModuleType("nfl_data_py") + module.import_pbp_data = import_pbp_data # type: ignore[attr-defined] + monkeypatch.setitem(sys.modules, "nfl_data_py", module) + + loader._nflverse_pbp(2025) + + assert received == {"seasons": [2025], "options": {"include_participation": False}} + + +def test_play_by_play_is_asked_for_without_the_participation_sidecar(nflverse): + """The sidecar is a separate release asset a season can lack while holding play-by-play. + + `import_pbp_data` reads both under one `try`, so requesting the sidecar makes its + absence discard the play-by-play beside it. + """ + fake = nflverse(import_pbp_data=source(2025)) + loader.load_pbp([2025]) + assert fake.calls == [("import_pbp_data", ([2025],), {"include_participation": False})] + + +def test_the_weekly_fallback_asks_for_play_by_play_without_the_sidecar(nflverse, pbp, rosters): + """The derivation reaches play-by-play under the same terms the direct loader does. + + nflverse publishes weekly stats per season after the fact, so a season in progress + reaches this path rather than the published asset. + """ + fake = nflverse( + import_weekly_data=unavailable, + import_pbp_data=pbp, + import_seasonal_rosters=rosters, + ) + loader.load_weekly_stats([2025]) + assert [call for call in fake.calls if call[0] == "import_pbp_data"] == [ + ("import_pbp_data", ([2025],), {"include_participation": False}) ] @@ -585,9 +643,15 @@ def test_a_request_holding_no_published_season_names_the_seasons(nflverse): loader.load_snap_counts([2025, 2026]) -def test_a_request_holding_no_published_season_names_the_source(nflverse): +def test_a_request_holding_no_published_season_blames_no_cause(nflverse): + """A dropped season names what was not obtained, not why. + + A season drops on a missing release asset and on a failed transfer alike, and the + loader cannot tell the two apart. Naming nflverse in the message would send a + reader to wait on a publication that may already have happened. + """ nflverse(import_snap_counts=source()) - with pytest.raises(RuntimeError, match="nflverse has not published"): + with pytest.raises(RuntimeError, match="^No snap counts obtained for seasons 2025, 2026$"): loader.load_snap_counts([2025, 2026]) @@ -653,9 +717,9 @@ def test_a_season_stored_by_a_partial_load_is_served_from_the_cache(nflverse): loader.load_snap_counts([2025, 2026]) loader.load_snap_counts([2025, 2026]) assert fake.calls == [ - ("import_snap_counts", ([2025],)), - ("import_snap_counts", ([2026],)), - ("import_snap_counts", ([2026],)), + ("import_snap_counts", ([2025],), {}), + ("import_snap_counts", ([2026],), {}), + ("import_snap_counts", ([2026],), {}), ] @@ -687,7 +751,7 @@ def test_weekly_stats_covered_by_neither_asset_in_any_season_raise(nflverse): import_pbp_data=source(absent=undefined_name), import_seasonal_rosters=source(), ) - with pytest.raises(RuntimeError, match="nflverse has not published"): + with pytest.raises(RuntimeError, match="obtained for"): loader.load_weekly_stats([2025, 2026]) diff --git a/tests/test_snap_share.py b/tests/test_snap_share.py index a10f0f7..8876e92 100644 --- a/tests/test_snap_share.py +++ b/tests/test_snap_share.py @@ -30,11 +30,33 @@ def test_compute_trends_filters_to_offensive(snap_counts): assert all(p in ("QB", "RB", "WR", "TE") for p in positions) -def test_compute_trends_drops_early_weeks(snap_counts): - """Weeks without enough history for velocity should be dropped.""" - trends = compute_trends(snap_counts, window=3) - assert trends["rolling_avg"].null_count() == 0 - assert trends["velocity"].null_count() == 0 +def test_the_week_opening_a_season_keeps_its_share_and_names_no_trend(colliding_name_snaps): + """A share is a measurement; a trend is a comparison, and week 1 has nothing to compare to. + + Dropping the week instead would take a season's opening weeks out of the frame the + filters are built from, so the season itself would stop being offered until its + third week. + """ + trends = compute_trends(colliding_name_snaps, window=3) + opening = trends.filter( + (pl.col("pfr_player_id") == COLLIDING_RISING_PFR_ID) & (pl.col("week") == 1) + ) + assert opening["snap_pct"].to_list() == [30.0] + assert opening["rolling_avg"].to_list() == [None] + assert opening["delta"].to_list() == [None] + assert opening["velocity"].to_list() == [None] + assert opening["trend"].to_list() == [None] + + +def test_the_second_week_slopes_against_the_only_week_behind_it(colliding_name_snaps): + """A window wider than the season is short-changed, not refused. + + The rising share climbs ten points a week, so week 2 slopes at ten over one week + just as the later weeks slope at ten over two. + """ + trends = compute_trends(colliding_name_snaps, window=3) + assert _by_week(trends, COLLIDING_RISING_PFR_ID, "velocity")[2] == pytest.approx(10.0) + assert _by_week(trends, COLLIDING_FALLING_PFR_ID, "velocity")[2] == pytest.approx(-10.0) def test_compute_trends_rising_player(snap_counts): @@ -62,6 +84,42 @@ def _by_week(trends: pl.DataFrame, pfr_id: str, column: str) -> dict: return dict(zip(rows["week"].to_list(), rows[column].to_list(), strict=True)) +def _measured(trends: pl.DataFrame) -> pl.DataFrame: + """The rows a trend was measured on — every week but the one opening a player's season.""" + return trends.drop_nulls(subset=["velocity"]) + + +def test_a_week_beyond_the_window_from_the_last_one_played_carries_no_trend( + colliding_name_snaps, +): + """A player back from a long absence has no recent series to slope across. + + `shift` counts appearances, so without a check on the weeks between them the row + returning from an absence would slope against whatever week came before it and + divide by one, reporting a whole absence's worth of change as one week of movement. + """ + gapped = colliding_name_snaps.filter(~pl.col("week").is_in([2, 3])) + trends = compute_trends(gapped, window=3) + returning = trends.filter( + (pl.col("pfr_player_id") == COLLIDING_RISING_PFR_ID) & (pl.col("week") == 4) + ) + assert returning["snap_pct"].to_list() == [60.0] + assert returning["velocity"].to_list() == [None] + assert returning["trend"].to_list() == [None] + + +def test_a_slope_divides_by_the_weeks_it_spans_not_the_rows(colliding_name_snaps): + """Two rows a week apart and two rows two weeks apart describe different speeds. + + The rising share climbs ten points a week, so every span reads +10 whether it is + measured across one week or two. + """ + gapped = colliding_name_snaps.filter(~pl.col("week").is_in([2, 3])) + velocities = _by_week(compute_trends(gapped, window=3), COLLIDING_RISING_PFR_ID, "velocity") + assert velocities[5] == pytest.approx(10.0) + assert velocities[6] == pytest.approx(10.0) + + def test_colliding_names_hold_separate_rolling_averages(colliding_name_snaps): """A rolling average covers one player's weeks, not every player wearing their name. @@ -70,53 +128,55 @@ def test_colliding_names_hold_separate_rolling_averages(colliding_name_snaps): then 30, 40 and 50 in week 4, and the falling player averages 80 and 70, then 80, 70 and 60. """ - trends = compute_trends(colliding_name_snaps, window=3) + trends = _measured(compute_trends(colliding_name_snaps, window=3)) assert _by_week(trends, COLLIDING_RISING_PFR_ID, "rolling_avg") == pytest.approx( - {3: 35.0, 4: 40.0, 5: 50.0, 6: 60.0} + {2: 30.0, 3: 35.0, 4: 40.0, 5: 50.0, 6: 60.0} ) assert _by_week(trends, COLLIDING_FALLING_PFR_ID, "rolling_avg") == pytest.approx( - {3: 75.0, 4: 70.0, 5: 60.0, 6: 50.0} + {2: 80.0, 3: 75.0, 4: 70.0, 5: 60.0, 6: 50.0} ) def test_colliding_names_hold_separate_deltas(colliding_name_snaps): """A delta measures a player's share against their own rolling average. - The rising player is 15 points over their week-3 average of 35 and 20 points over - every later one; the falling player is under theirs by the same margins. + The rising player is 10 points over their week-2 average of 30, 15 points over + their week-3 average of 35 and 20 points over every later one; the falling player + is under theirs by the same margins. """ - trends = compute_trends(colliding_name_snaps, window=3) + trends = _measured(compute_trends(colliding_name_snaps, window=3)) assert _by_week(trends, COLLIDING_RISING_PFR_ID, "delta") == pytest.approx( - {3: 15.0, 4: 20.0, 5: 20.0, 6: 20.0} + {2: 10.0, 3: 15.0, 4: 20.0, 5: 20.0, 6: 20.0} ) assert _by_week(trends, COLLIDING_FALLING_PFR_ID, "delta") == pytest.approx( - {3: -15.0, 4: -20.0, 5: -20.0, 6: -20.0} + {2: -10.0, 3: -15.0, 4: -20.0, 5: -20.0, 6: -20.0} ) def test_colliding_names_hold_separate_velocities(colliding_name_snaps): """A velocity is the slope of one player's own shares across the window. - Each series moves ten points a week, so the two-week slope is +10 for the rising - player and -10 for the falling one in every week the window covers. + Each series moves ten points a week, so the slope is +10 for the rising player and + -10 for the falling one in every week the window covers, whether it spans one week + or the full two. """ - trends = compute_trends(colliding_name_snaps, window=3) + trends = _measured(compute_trends(colliding_name_snaps, window=3)) assert _by_week(trends, COLLIDING_RISING_PFR_ID, "velocity") == pytest.approx( - dict.fromkeys(range(3, 7), 10.0) + dict.fromkeys(range(2, 7), 10.0) ) assert _by_week(trends, COLLIDING_FALLING_PFR_ID, "velocity") == pytest.approx( - dict.fromkeys(range(3, 7), -10.0) + dict.fromkeys(range(2, 7), -10.0) ) def test_colliding_names_classify_in_opposite_directions(colliding_name_snaps): """Each player wearing one name lands on the trend their own shares describe.""" - trends = compute_trends(colliding_name_snaps, window=3) + trends = _measured(compute_trends(colliding_name_snaps, window=3)) assert _by_week(trends, COLLIDING_RISING_PFR_ID, "trend") == dict.fromkeys( - range(3, 7), "rising" + range(2, 7), "rising" ) assert _by_week(trends, COLLIDING_FALLING_PFR_ID, "trend") == dict.fromkeys( - range(3, 7), "falling" + range(2, 7), "falling" ) diff --git a/tests/test_stats.py b/tests/test_stats.py index 5a0f055..16bb858 100644 --- a/tests/test_stats.py +++ b/tests/test_stats.py @@ -54,6 +54,7 @@ "rusher_player_name": pl.String, "fumbled_1_player_id": pl.String, "fumbled_1_player_name": pl.String, + "fumbled_1_team": pl.String, } #: Columns that count events or yardage, and so read zero rather than null on a @@ -186,6 +187,7 @@ def carry( fields["fumble_lost"] = 1.0 fields["fumbled_1_player_id"] = fumbler.player_id fields["fumbled_1_player_name"] = fumbler.name + fields["fumbled_1_team"] = offense return play(**fields) @@ -500,14 +502,33 @@ def test_omitting_rosters_nulls_the_position_column_and_leaves_the_rest( # ── Fumbles away from scrimmage ────────────────────────────────────────────── -def return_fumble(fumbler: Player, *, play_type: str) -> dict[str, object]: - """One `play_type` snap that `fumbler` loses on a fumble.""" +def return_fumble(fumbler: Player, *, play_type: str, team: str = HOME) -> dict[str, object]: + """One `play_type` snap that `fumbler`, of `team`, loses on a fumble. + + A return is the one play where the fumbler's team is not the team with the ball, so + `team` and `posteam` are set apart. + """ return play( play_type=play_type, fumble_lost=1.0, fumbled_1_player_id=fumbler.player_id, fumbled_1_player_name=fumbler.name, + fumbled_1_team=team, + ) + + +def test_a_fumble_on_a_return_is_charged_to_the_fumblers_own_team() -> None: + """`posteam` names the kicking team on a return, and the returner is not on it. + + Charging the fumble to `posteam` would give the returner a second weekly line under + a team they never played for, which every join keyed on the team then doubles. + """ + stats = compute_weekly_stats_from_pbp( + frame([return_fumble(CARRIER, play_type="punt", team=AWAY)]) ) + line = rows_for(stats, CARRIER) + assert line.height == 1 + assert line["recent_team"].item() == AWAY @pytest.mark.parametrize("play_type", ["kickoff", "punt"]) @@ -558,6 +579,27 @@ def spelled(role: str) -> Player: ] +def test_a_player_spelled_differently_within_one_role_yields_one_line() -> None: + """Identity is the id inside a role as much as across roles. + + nflverse spells a name from the source field of each play, so two plays in one week + can name one receiver differently. Grouping on the spelling would split the week + into two partial lines, each carrying part of the targets. + """ + stats = compute_weekly_stats_from_pbp( + frame( + [ + throw(PASSER, CATCHER, yards=10.0), + throw(PASSER, Player(CATCHER.player_id, "B.Lindqvist"), yards=20.0), + ] + ) + ) + line = rows_for(stats, CATCHER) + assert line.height == 1 + assert line["targets"].item() == 2.0 + assert line["receiving_yards"].item() == 30.0 + + def test_a_player_spelled_differently_across_roles_yields_one_line() -> None: stats = compute_weekly_stats_from_pbp(frame(DISAGREEING_NAMES_WEEK)) assert rows_for(stats, ALL_PURPOSE).height == 1 diff --git a/tests/test_ui_base.py b/tests/test_ui_base.py index da1b3ef..ee58860 100644 --- a/tests/test_ui_base.py +++ b/tests/test_ui_base.py @@ -1005,16 +1005,116 @@ async def test_start_sit_says_so_when_no_season_carries_a_completed_week( @piloted -async def test_trade_value_says_so_when_no_season_reaches_the_week_a_ranking_rests_on( +async def test_trade_value_offers_a_season_too_few_weeks_deep_to_rank( monkeypatch: pytest.MonkeyPatch, fixture_frames: dict[str, pl.DataFrame], ) -> None: - """A season fewer weeks deep than the floor ranks nobody, and the table says why.""" + """A season the ranking cannot yet run on is still the season being played. + + Withholding it from the filter would leave the season before it selected, and a + finished season's final chart would read as the live one. + """ + weekly = fixture_frames["load_weekly_stats"] + played = weekly.filter(pl.col("week") < trade_value_screen.MIN_WEEKS_PLAYED) + frames_for(monkeypatch, played, fixture_frames["load_schedules"], fixture_frames) + latest = max(played["season"].unique().drop_nulls().to_list()) + + view = trade_value_screen.TradeValueView() + async with Host(view).run_test() as pilot: + view.activate() + await settle(pilot) + + assert view.query_one("#tv-filter-season", Select).value == latest + + +@piloted +async def test_trade_value_says_why_the_selected_season_ranks_nobody( + monkeypatch: pytest.MonkeyPatch, + fixture_frames: dict[str, pl.DataFrame], +) -> None: + """A season fewer weeks deep than the floor ranks nobody, and the table names it.""" + weekly = fixture_frames["load_weekly_stats"] + played = weekly.filter(pl.col("week") < trade_value_screen.MIN_WEEKS_PLAYED) + frames_for(monkeypatch, played, fixture_frames["load_schedules"], fixture_frames) + latest = max(played["season"].unique().drop_nulls().to_list()) + + view = trade_value_screen.TradeValueView() + async with Host(view).run_test() as pilot: + view.activate() + await settle(pilot) + + assert view.query_one("#tv-table", DataTable).row_count == 0 + assert str(view.query_one("#tv-table-empty", Static).visual) == ( + f"No trade values for {latest}. A ranking rests on " + f"{trade_value_screen.MIN_WEEKS_PLAYED} completed weeks " + f"and {latest} carries fewer." + ) + + +@piloted +async def test_trade_value_asks_for_a_season_when_the_filter_is_cleared( + monkeypatch: pytest.MonkeyPatch, + fixture_frames: dict[str, pl.DataFrame], +) -> None: + """An empty filter is a choice not yet made, not a season the data is missing. + + Reporting it as absent data would have the table contradict the season filter + standing beside it, which still lists every loaded season. + """ frames_for( monkeypatch, - fixture_frames["load_weekly_stats"].filter( - pl.col("week") < trade_value_screen.MIN_WEEKS_PLAYED - ), + fixture_frames["load_weekly_stats"], + fixture_frames["load_schedules"], + fixture_frames, + ) + view = trade_value_screen.TradeValueView() + async with Host(view).run_test() as pilot: + view.activate() + await settle(pilot) + + view.query_one("#tv-filter-season", Select).clear() + await settle(pilot) + + assert ( + str(view.query_one("#tv-table-empty", Static).visual) + == trade_value_screen.NO_SEASON_SELECTED + ) + + +@piloted +async def test_trade_value_drops_the_week_options_when_the_season_is_cleared( + monkeypatch: pytest.MonkeyPatch, + fixture_frames: dict[str, pl.DataFrame], +) -> None: + """A week belongs to a season, so no season leaves no week to offer.""" + frames_for( + monkeypatch, + fixture_frames["load_weekly_stats"], + fixture_frames["load_schedules"], + fixture_frames, + ) + view = trade_value_screen.TradeValueView() + async with Host(view).run_test() as pilot: + view.activate() + await settle(pilot) + assert view.selected("tv-filter-week") is not None + + view.query_one("#tv-filter-season", Select).clear() + await settle(pilot) + + assert view.selected("tv-filter-week") is None + + +@piloted +async def test_trade_value_says_so_when_no_season_carries_a_played_week( + monkeypatch: pytest.MonkeyPatch, + fixture_frames: dict[str, pl.DataFrame], +) -> None: + """With no season played at all there is none to name, so the table says only that.""" + weekly = fixture_frames["load_weekly_stats"] + frames_for( + monkeypatch, + weekly.filter(pl.col("season_type") != "REG"), fixture_frames["load_schedules"], fixture_frames, ) @@ -1026,5 +1126,5 @@ async def test_trade_value_says_so_when_no_season_reaches_the_week_a_ranking_res assert view.query_one("#tv-table", DataTable).row_count == 0 assert ( str(view.query_one("#tv-table-empty", Static).visual) - == trade_value_screen.NO_RANKABLE_WEEKS + == trade_value_screen.NO_PLAYED_WEEKS ) diff --git a/tests/test_waiver_wire.py b/tests/test_waiver_wire.py index 20b7ffd..88775b4 100644 --- a/tests/test_waiver_wire.py +++ b/tests/test_waiver_wire.py @@ -37,10 +37,54 @@ def _by_week(trends: pl.DataFrame, gsis_id: str, column: str) -> dict: return dict(zip(rows["week"].to_list(), rows[column].to_list(), strict=True)) +def _measured(trends: pl.DataFrame) -> pl.DataFrame: + """The rows a trend was measured on — every week but the one opening a player's season.""" + return trends.drop_nulls(subset=["velocity"]) + + def _colliding_trends(snaps, stats, ids): return compute_usage_trends(snaps, stats, ids, window=3) +def test_the_week_opening_a_season_keeps_its_usage_and_names_no_trend( + colliding_name_snaps, colliding_name_weekly_stats, player_ids +): + """Usage is a measurement; a trend is a comparison, and week 1 has nothing to compare to. + + Dropping the week instead would take a season's opening weeks out of the frame the + filters are built from, so the season itself would stop being offered until its + third week. + """ + trends = _colliding_trends(colliding_name_snaps, colliding_name_weekly_stats, player_ids) + opening = trends.filter((pl.col("gsis_id") == COLLIDING_RISING_GSIS_ID) & (pl.col("week") == 1)) + assert opening["usage_score"].to_list() == pytest.approx([72.0]) + assert opening["rolling_avg"].to_list() == [None] + assert opening["delta"].to_list() == [None] + assert opening["velocity"].to_list() == [None] + assert opening["trend"].to_list() == [None] + + +def test_a_week_beyond_the_window_from_the_last_one_played_carries_no_trend( + colliding_name_snaps, colliding_name_weekly_stats, player_ids +): + """A player back from a long absence has no recent series to slope across. + + `shift` counts appearances, so without a check on the weeks between them the row + returning from an absence would slope against whatever week came before it and + divide by one, reporting a whole absence's worth of change as one week of movement. + """ + trends = _colliding_trends( + colliding_name_snaps.filter(~pl.col("week").is_in([2, 3])), + colliding_name_weekly_stats, + player_ids, + ) + returning = trends.filter( + (pl.col("gsis_id") == COLLIDING_RISING_GSIS_ID) & (pl.col("week") == 4) + ) + assert returning["velocity"].to_list() == [None] + assert returning["trend"].to_list() == [None] + + def test_colliding_names_hold_separate_usage_scores( colliding_name_snaps, colliding_name_weekly_stats, player_ids ): @@ -53,10 +97,10 @@ def test_colliding_names_hold_separate_usage_scores( """ trends = _colliding_trends(colliding_name_snaps, colliding_name_weekly_stats, player_ids) assert _by_week(trends, COLLIDING_RISING_GSIS_ID, "usage_score") == pytest.approx( - {3: 80.0, 4: 84.0, 5: 88.0, 6: 92.0} + {1: 72.0, 2: 76.0, 3: 80.0, 4: 84.0, 5: 88.0, 6: 92.0} ) assert _by_week(trends, COLLIDING_FALLING_GSIS_ID, "usage_score") == pytest.approx( - {3: 49.0, 4: 45.0, 5: 41.0, 6: 37.0} + {1: 57.0, 2: 53.0, 3: 49.0, 4: 45.0, 5: 41.0, 6: 37.0} ) @@ -66,15 +110,17 @@ def test_colliding_names_hold_separate_rolling_averages( """A rolling average covers one player's weeks, not every player wearing their name. The rising scores read 72, 76, 80, 84, 88, 92, so the average of the preceding - weeks is 74 in week 3 and 76 in week 4. The falling scores read 57, 53, 49, 45, - 41, 37, giving 55 and 53. + weeks is 72 in week 2, 74 in week 3 and 76 in week 4. The falling scores read 57, + 53, 49, 45, 41, 37, giving 57, 55 and 53. """ - trends = _colliding_trends(colliding_name_snaps, colliding_name_weekly_stats, player_ids) + trends = _measured( + _colliding_trends(colliding_name_snaps, colliding_name_weekly_stats, player_ids) + ) assert _by_week(trends, COLLIDING_RISING_GSIS_ID, "rolling_avg") == pytest.approx( - {3: 74.0, 4: 76.0, 5: 80.0, 6: 84.0} + {2: 72.0, 3: 74.0, 4: 76.0, 5: 80.0, 6: 84.0} ) assert _by_week(trends, COLLIDING_FALLING_GSIS_ID, "rolling_avg") == pytest.approx( - {3: 55.0, 4: 53.0, 5: 49.0, 6: 45.0} + {2: 57.0, 3: 55.0, 4: 53.0, 5: 49.0, 6: 45.0} ) @@ -83,15 +129,18 @@ def test_colliding_names_hold_separate_deltas( ): """A delta measures a player's score against their own rolling average. - The rising player is 6 points over their week-3 average of 74 and 8 points over - every later one; the falling player is under theirs by the same margins. + The rising player is 4 points over their week-2 average of 72, 6 points over their + week-3 average of 74 and 8 points over every later one; the falling player is under + theirs by the same margins. """ - trends = _colliding_trends(colliding_name_snaps, colliding_name_weekly_stats, player_ids) + trends = _measured( + _colliding_trends(colliding_name_snaps, colliding_name_weekly_stats, player_ids) + ) assert _by_week(trends, COLLIDING_RISING_GSIS_ID, "delta") == pytest.approx( - {3: 6.0, 4: 8.0, 5: 8.0, 6: 8.0} + {2: 4.0, 3: 6.0, 4: 8.0, 5: 8.0, 6: 8.0} ) assert _by_week(trends, COLLIDING_FALLING_GSIS_ID, "delta") == pytest.approx( - {3: -6.0, 4: -8.0, 5: -8.0, 6: -8.0} + {2: -4.0, 3: -6.0, 4: -8.0, 5: -8.0, 6: -8.0} ) @@ -100,15 +149,18 @@ def test_colliding_names_hold_separate_velocities( ): """A velocity is the slope of one player's own scores across the window. - Each score moves four points a week, so the two-week slope is +4 for the rising - player and -4 for the falling one in every week the window covers. + Each score moves four points a week, so the slope is +4 for the rising player and + -4 for the falling one in every week the window covers, whether it spans one week + or the full two. """ - trends = _colliding_trends(colliding_name_snaps, colliding_name_weekly_stats, player_ids) + trends = _measured( + _colliding_trends(colliding_name_snaps, colliding_name_weekly_stats, player_ids) + ) assert _by_week(trends, COLLIDING_RISING_GSIS_ID, "velocity") == pytest.approx( - dict.fromkeys(range(3, 7), 4.0) + dict.fromkeys(range(2, 7), 4.0) ) assert _by_week(trends, COLLIDING_FALLING_GSIS_ID, "velocity") == pytest.approx( - dict.fromkeys(range(3, 7), -4.0) + dict.fromkeys(range(2, 7), -4.0) ) @@ -116,12 +168,14 @@ def test_colliding_names_classify_in_opposite_directions( colliding_name_snaps, colliding_name_weekly_stats, player_ids ): """Each player wearing one name lands on the trend their own usage describes.""" - trends = _colliding_trends(colliding_name_snaps, colliding_name_weekly_stats, player_ids) + trends = _measured( + _colliding_trends(colliding_name_snaps, colliding_name_weekly_stats, player_ids) + ) assert _by_week(trends, COLLIDING_RISING_GSIS_ID, "trend") == dict.fromkeys( - range(3, 7), "rising" + range(2, 7), "rising" ) assert _by_week(trends, COLLIDING_FALLING_GSIS_ID, "trend") == dict.fromkeys( - range(3, 7), "falling" + range(2, 7), "falling" ) @@ -142,5 +196,5 @@ def test_a_zero_target_team_gives_its_players_no_target_share( """A share over a zero team total reads zero rather than dividing by nothing.""" trends = _colliding_trends(colliding_name_snaps, colliding_name_weekly_stats, player_ids) assert _by_week(trends, COLLIDING_FALLING_GSIS_ID, "tgt_share") == pytest.approx( - dict.fromkeys(range(3, 7), 0.0) + dict.fromkeys(range(1, 7), 0.0) )