docs: streamline README and reorganize v1.1 documentation (#306) - #328
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| import httpx | ||
|
|
||
| from mlbstatsapi import AsyncMlb | ||
|
|
||
|
|
||
| client = httpx.AsyncClient() | ||
| try: | ||
| async with AsyncMlb(client=client) as mlb: | ||
| player = await mlb.get_person(664034) | ||
| finally: | ||
| await client.aclose() |
There was a problem hiding this comment.
Bug: this example uses async with / await at module top level, outside any async def. As written this raises SyntaxError: 'async with' outside async function if copy-pasted into a script. Every other example in this file correctly wraps its body in async def main(): ... + asyncio.run(main()) — see
python-mlb-statsapi/docs/async.md
Lines 1 to 20 in 77c0f7b
| import httpx | |
| from mlbstatsapi import AsyncMlb | |
| client = httpx.AsyncClient() | |
| try: | |
| async with AsyncMlb(client=client) as mlb: | |
| player = await mlb.get_person(664034) | |
| finally: | |
| await client.aclose() | |
| import asyncio | |
| import httpx | |
| from mlbstatsapi import AsyncMlb | |
| async def main(): | |
| client = httpx.AsyncClient() | |
| try: | |
| async with AsyncMlb(client=client) as mlb: | |
| player = await mlb.get_person(664034) | |
| finally: | |
| await client.aclose() | |
| asyncio.run(main()) |
There was a problem hiding this comment.
How should I handle this?
There was a problem hiding this comment.
@claude I would like a set of examples to send to the user. Wrapping around main isn't practical IMO.
There was a problem hiding this comment.
Claude finished @Mattsface's task in 2m 4s —— View job
Done
- Read the trigger comment and surrounding review thread
- Look at
docs/async.mdto see the current example conventions - Draft a set of alternative example formats that avoid full
async def main(): ... asyncio.run(main())wrapping on every snippet - Applied the set to
docs/async.mdand pushed
Context
Commit 7bef313 already reworked the "Custom HTTPX client" section so the reusable logic (get_person_with_custom_client) is a plain async def function with no opinion on how it's invoked, rather than being baked into a main(). That addressed the immediate SyntaxError bug, but only showed one invocation style (asyncio.run(main())).
Since you'd rather not force every advanced example through a main() wrapper, I expanded that section into four concrete "how to actually call this" patterns readers can pick from depending on their app, pushed as 7f5b796:
- Script entry point — the original
asyncio.run(main())pattern, kept as the "just run it" option. - Already inside a running event loop (web framework handler, worker task, etc.) — just
awaitit directly with a client the app already owns; nomain()at all. - FastAPI (or another ASGI framework) — a concrete route handler showing the client injected once at app scope and reused per-request.
- Interactive / notebook use —
pycon-style snippet noting that Jupyter/IPython andpython -m asynciosupport top-levelawait, so no wrapper is needed there either.
The closing paragraph about client ownership was tweaked from "the entry point above" to "the examples above" since there are now several.
I kept this scoped to the "Custom HTTPX client" section, since that's where the main()-wrapping complaint was raised — the earlier sections (Quick start, Without a context manager, Concurrent requests, Error handling) still use the simple async def main(): ... asyncio.run(main()) pattern for their first, "getting started" example, which seems fine to leave as the default on-ramp. Let me know if you'd like the same multi-pattern treatment applied there too, or if any of the four new options above should be trimmed/adjusted.
Note: the "Supported endpoints" list further up in docs/async.md (and the matching claim in README.md) is still flagged as stale/incorrect by several other review comments in this thread — that's a separate issue from this one and wasn't touched here.
| ## Supported endpoints | ||
|
|
||
| The async surface is intentionally smaller than the synchronous `Mlb` surface | ||
| while 1.1 support is being expanded. The currently supported awaitable endpoint | ||
| methods on `release/1.1.0` are: | ||
|
|
||
| ```text | ||
| get_team(...) | ||
| get_teams(...) | ||
| get_person(...) | ||
| get_people(...) | ||
| get_schedule(...) | ||
| ``` | ||
|
|
||
| Where an async endpoint is supported, it returns the same Pydantic model types | ||
| and follows the same public HTTP/error behavior as the matching synchronous | ||
| method. | ||
|
|
||
| For the authoritative list and signatures, see the | ||
| [public API contract](public-api.md#asyncmlb-public-client). |
There was a problem hiding this comment.
Bug: this "Supported endpoints" list is drastically incomplete and contradicts the document it calls authoritative a few lines below. AsyncMlb (mlbstatsapi/async_mlb.py) actually defines ~40 public get_* async methods (e.g. get_team_roster, get_game, get_player_stats, get_standings, get_draft, get_awards, get_gamepace, …), not just the 5 listed here. docs/public-api.md states outright that "AsyncMlb now covers every endpoint method Mlb exposes" — directly contradicting the "intentionally smaller" framing and the 5-item list in this section.
This needs a structural fix rather than a one-line suggestion: either drop the enumerated list and framing sentence and state that AsyncMlb mirrors Mlb's endpoint surface (with aclose() in place of close()), deferring to public-api.md#asyncmlb-public-client, or update the list to match reality.
There was a problem hiding this comment.
I'm not sure how this bug appeared. Let me go check.
| >>> season_hitting = stats['hitting']['season'] | ||
| >>> advanced_hitting = stats['hitting']['seasonAdvanced'] | ||
| ``` | ||
| Higher-level stats helpers remain on the synchronous `Mlb` client in the current 1.1 async surface. |
There was a problem hiding this comment.
Bug: this claim is false. AsyncMlb.get_player_stats, get_team_stats, get_stats, and get_players_stats_for_game all exist as real async def methods in mlbstatsapi/async_mlb.py (e.g. lines 2029, 2085, 2133, 2192), fully implemented (not stubs) and documented as the "Async counterpart of Mlb.get_*". This would mislead async users into thinking they need the sync client for stats.
| Higher-level stats helpers remain on the synchronous `Mlb` client in the current 1.1 async surface. | |
| `AsyncMlb` exposes the same stats helpers as awaitables (e.g. `get_player_stats`, `get_team_stats`). |
There was a problem hiding this comment.
Let's remove this. Thanks for catching it.
| ## Supported endpoints | ||
|
|
||
| The async surface is intentionally smaller than the synchronous `Mlb` surface | ||
| while 1.1 support is being expanded. The currently supported awaitable endpoint | ||
| methods on `release/1.1.0` are: | ||
|
|
||
| ```text | ||
| get_team(...) | ||
| get_teams(...) | ||
| get_person(...) | ||
| get_people(...) | ||
| get_schedule(...) | ||
| ``` |
There was a problem hiding this comment.
This "5 supported methods" list appears to be inaccurate and contradicts the pre-existing, authoritative docs/public-api.md, which lists ~38 async endpoint methods as currently supported (including get_team_stats, get_player_stats, get_stats, get_draft, get_awards, get_game, etc.). This is confirmed by mlbstatsapi/async_mlb.py, which defines async def for all of those methods.
The same inaccuracy is repeated in README.md line 202: "Higher-level stats helpers remain on the synchronous Mlb client in the current 1.1 async surface" — but get_player_stats, get_team_stats, and get_stats are all implemented as async methods per public-api.md and the source.
Since this PR's own description says method names/examples were checked against the current public API documentation, this list likely needs to be brought in line with public-api.md (or that section should just point readers to public-api.md for the current list instead of duplicating a stale one).
| ## Supported endpoints | ||
|
|
||
| The async surface is intentionally smaller than the synchronous `Mlb` surface | ||
| while 1.1 support is being expanded. The currently supported awaitable endpoint | ||
| methods on `release/1.1.0` are: | ||
|
|
||
| ```text | ||
| get_team(...) | ||
| get_teams(...) | ||
| get_person(...) | ||
| get_people(...) | ||
| get_schedule(...) | ||
| ``` | ||
|
|
||
| Where an async endpoint is supported, it returns the same Pydantic model types | ||
| and follows the same public HTTP/error behavior as the matching synchronous | ||
| method. | ||
|
|
||
| For the authoritative list and signatures, see the | ||
| [public API contract](public-api.md#asyncmlb-public-client). |
There was a problem hiding this comment.
Bug: "Supported endpoints" list contradicts this same PR and the actual source.
This section states the async surface is "intentionally smaller" and lists only get_team, get_teams, get_person, get_people, and get_schedule as supported. That's inconsistent with:
docs/stats.md(added in this same PR), which shows workingAsyncMlbexamples forget_player_stats,get_team_stats,get_stats, andget_players_stats_for_game— none of which are in this "supported" list.mlbstatsapi/async_mlb.py, which currently defines ~40async def get_*methods, not 5.docs/public-api.md(cited here as authoritative — see public-api.md#L381), which states "AsyncMlbnow covers every endpoint methodMlbexposes."
A reader following this section would wrongly conclude most async endpoints (including all stats methods) aren't supported. Since this needs correct information about the full async surface rather than a mechanical text swap, I'd suggest reworking this section to state that AsyncMlb mirrors the full Mlb endpoint surface (differing only in close() vs aclose()) and pointing to public-api.md as the source of truth, rather than enumerating a stale subset.
python-mlb-statsapi/docs/async.md
Lines 91 to 112 in dbe46e7
| ## Supported endpoints | ||
|
|
||
| The async surface is intentionally smaller than the synchronous `Mlb` surface | ||
| while 1.1 support is being expanded. The currently supported awaitable endpoint | ||
| methods on `release/1.1.0` are: | ||
|
|
||
| ```text | ||
| get_team(...) | ||
| get_teams(...) | ||
| get_person(...) | ||
| get_people(...) | ||
| get_schedule(...) | ||
| ``` |
There was a problem hiding this comment.
Bug: this "Supported endpoints" list contradicts docs/stats.md, and appears to be the stale/wrong side of the contradiction.
This section claims the only currently-awaitable AsyncMlb methods are get_team, get_teams, get_person, get_people, and get_schedule. But the new docs/stats.md added in this same PR gives full AsyncMlb examples for get_player_stats, get_team_stats, get_stats, and get_players_stats_for_game, and states "Both Mlb and AsyncMlb return the same structure" / "The synchronous and asynchronous signatures match." The README's new "Common Methods → Stats" section also links to the stats guide promising "sync and async examples."
Checking mlbstatsapi/async_mlb.py on this branch, AsyncMlb actually implements roughly 40 awaitable methods, including all four stats methods — matching docs/public-api.md#asyncmlb-public-client, which this very section links to six lines below as "the authoritative list." So the hardcoded 5-method list here is stale/incorrect, and following it would incorrectly tell readers that every AsyncMlb example in docs/stats.md is unsupported.
Suggested fix: replace the hardcoded list with a pointer to public-api.md#asyncmlb-public-client (as this file already does for signatures), or update it to match the actual async surface, and drop the "intentionally smaller ... while 1.1 support is being expanded" framing.
python-mlb-statsapi/docs/async.md
Lines 91 to 105 in dbe46e7
| | `get_scheduled_games_by_date()` | Return scheduled games from dates | | ||
|
|
||
| ```text | ||
| Mlb.get_schedule(date: str, start_date: str, end_date: str, sport_id: int, team_id: int, **params) |
There was a problem hiding this comment.
Bug: get_schedule is listed twice with contradictory signatures.
This code block documents Mlb.get_schedule twice back-to-back — once with all parameters required (this line), once with all parameters defaulted (the next line). Python doesn't support overloading, so only one can be correct. This is a leftover artifact from the old README, which had two separate "Schedules" sections that got merged here without deduplicating.
mlbstatsapi/mlb_api.py defines a single get_schedule with every parameter defaulted (date: str = None, start_date: str = None, end_date: str = None, sport_id: int = 1, team_id: int = None), matching the second line, not this one. This required-args line is stale and misleading — it implies get_schedule() can't be called with no arguments, but it can (it returns today's schedule).
python-mlb-statsapi/docs/methods.md
Lines 91 to 97 in dbe46e7
| Mlb.get_schedule(date: str, start_date: str, end_date: str, sport_id: int, team_id: int, **params) |
| ## Common Methods | ||
|
|
||
| ### Pull Request Guidelines | ||
| ### Players | ||
|
|
||
| - Run offline tests before submitting a PR | ||
| - Use the [PR template](.github/pull_request_template.md) when creating your pull request | ||
| - Follow the branch naming convention: | ||
| - `feat/` - New features | ||
| - `fix/` - Bug fixes | ||
| - `docs/` - Documentation updates | ||
| - `refactor/` - Code improvements | ||
|
|
||
| ### Reporting Issues | ||
|
|
||
| Found a bug or have a feature request? Please [open an issue](https://github.com/zero-sum-seattle/python-mlb-statsapi/issues/new) with: | ||
|
|
||
| - A clear description of the problem or feature | ||
| - Steps to reproduce (for bugs) | ||
| - Expected vs actual behavior | ||
| - Python version and package version | ||
|
|
||
|
|
||
| ## Examples | ||
|
|
||
| Let's show some examples of getting stat objects from the API. What is baseball without stats, right? | ||
|
|
||
| ### Player Stats | ||
| Get the Id(s) of the players you want stats for and set stat types and groups. | ||
| ```python | ||
| >>> mlb = mlbstatsapi.Mlb() | ||
| >>> player_id = mlb.get_people_id("Ty France")[0] | ||
| >>> stats = ['season', 'career'] | ||
| >>> groups = ['hitting', 'pitching'] | ||
| >>> params = {'season': 2022} | ||
| player = mlb.get_person(664034) | ||
| players = mlb.get_people() | ||
| player_ids = mlb.get_people_id("Ty France") | ||
| ``` | ||
|
|
||
| Use player id with stat types and groups to return a stats dictionary | ||
| ```python | ||
| >>> stat_dict = mlb.get_player_stats(player_id, stats=stats, groups=groups, **params) | ||
| >>> season_hitting_stat = stat_dict['hitting']['season'] | ||
| >>> career_pitching_stat = stat_dict['pitching']['career'] | ||
| ``` | ||
| ### Teams | ||
|
|
||
| Print season hitting stats using Pydantic's `model_dump()` | ||
| ```python | ||
| >>> for split in season_hitting_stat.splits: | ||
| ... print(split.stat.model_dump(exclude_none=True)) | ||
| {'games_played': 140, 'groundouts': 163, 'airouts': 148, 'runs': 65, 'doubles': 27, ...} | ||
| team = mlb.get_team(136) | ||
| teams = mlb.get_teams() | ||
| team_ids = mlb.get_team_id("Seattle Mariners") | ||
| ``` | ||
|
|
||
| Or access individual fields directly | ||
| ```python | ||
| >>> for split in season_hitting_stat.splits: | ||
| ... print(f"Games: {split.stat.games_played}") | ||
| ... print(f"Home Runs: {split.stat.home_runs}") | ||
| ... print(f"Batting Avg: {split.stat.avg}") | ||
| Games: 140 | ||
| Home Runs: 20 | ||
| Batting Avg: .274 | ||
| ``` | ||
| ### Stats | ||
|
|
||
| ### Team Stats | ||
| Get the Team Id(s) | ||
| ```python | ||
| >>> mlb = mlbstatsapi.Mlb() | ||
| >>> team_id = mlb.get_team_id('Seattle Mariners')[0] | ||
| ``` | ||
| The stats API has several entry points and returns a nested `stats[group][type]` structure. See the dedicated [Stats Guide](docs/stats.md) for `get_player_stats()`, `get_team_stats()`, `get_stats()`, and `get_players_stats_for_game()` examples using both `Mlb` and `AsyncMlb`. | ||
|
|
||
| Set the stat types and groups | ||
| ```python | ||
| >>> stats = ['season', 'seasonAdvanced'] | ||
| >>> groups = ['hitting'] | ||
| >>> params = {'season': 2022} | ||
| ``` | ||
|
|
||
| Use team id and the stat types and groups to return season hitting stats | ||
| ```python | ||
| >>> stats = mlb.get_team_stats(team_id, stats=stats, groups=groups, **params) | ||
| >>> season_hitting = stats['hitting']['season'] | ||
| >>> advanced_hitting = stats['hitting']['seasonAdvanced'] | ||
| ``` | ||
|
|
||
| Print stats as JSON | ||
| ```python | ||
| >>> for split in season_hitting.splits: | ||
| ... print(split.stat.model_dump_json(indent=2, exclude_none=True)) | ||
| { | ||
| "games_played": 162, | ||
| "groundouts": 1273, | ||
| "runs": 690, | ||
| "doubles": 229, | ||
| ... | ||
| } | ||
| ``` | ||
| ### Schedule | ||
|
|
||
| ### Expected Stats | ||
| ```python | ||
| >>> player_id = mlb.get_people_id('Ty France')[0] | ||
| >>> stats = ['expectedStatistics'] | ||
| >>> group = ['hitting'] | ||
| >>> params = {'season': 2022} | ||
|
|
||
| >>> stats = mlb.get_player_stats(player_id, stats=stats, groups=group, **params) | ||
| >>> expected = stats['hitting']['expectedStatistics'] | ||
| >>> for split in expected.splits: | ||
| ... print(f"Expected AVG: {split.stat.avg}") | ||
| ... print(f"Expected SLG: {split.stat.slg}") | ||
| Expected AVG: .259 | ||
| Expected SLG: .394 | ||
| schedule = mlb.get_schedule(date="2022-10-13") | ||
| ``` |
There was a problem hiding this comment.
Bug: the Players/Teams/Schedule snippets in "Common Methods" use mlb without ever defining it.
Each of these three code blocks starts directly with a call like player = mlb.get_person(664034), team = mlb.get_team(136), or schedule = mlb.get_schedule(date="2022-10-13"), with no from mlbstatsapi import Mlb and no client construction (with Mlb() as mlb: or similar). Copy-pasting any of these blocks raises NameError: name 'mlb' is not defined.
The nearest preceding code is the "Concurrent Async Requests" example above, but mlb there is bound only inside async def main() via async with AsyncMlb() as mlb: — it's function-local and out of scope by the time these snippets appear. This is also inconsistent with every other Python snippet added in this PR (Quick Start, docs/examples.md, docs/stats.md), which all include the import and client setup.
Suggested fix: prefix each snippet with from mlbstatsapi import Mlb and with Mlb() as mlb: (or add one shared setup block at the top of "Common Methods" and note that the snippets below assume it).
Lines 173 to 199 in dbe46e7
| Mlb.get_schedule(date: str, start_date: str, end_date: str, sport_id: int, team_id: int, **params) | ||
| Mlb.get_schedule(date: str = None, start_date: str = None, end_date: str = None, sport_id: int = 1, team_id: int = None, **params) | ||
| Mlb.get_scheduled_games_by_date(date: str = None, start_date: str = None, end_date: str = None, sport_id: int = 1, **params) |
There was a problem hiding this comment.
Bug: Mlb.get_schedule is listed twice in this code block with mutually contradictory signatures — one with all parameters required, one with all parameters defaulted. Only the second (all-defaulted) form matches the actual implementation (mlbstatsapi/mlb_api.py, get_schedule(self, date: str = None, start_date: str = None, end_date: str = None, sport_id: int = 1, team_id: int = None, **params)), and it's the only form consistent with the usage examples elsewhere in this PR (e.g. mlb.get_schedule(date="2022-10-13") in README.md and docs/examples.md).
python-mlb-statsapi/docs/methods.md
Lines 91 to 97 in dbe46e7
| Mlb.get_schedule(date: str, start_date: str, end_date: str, sport_id: int, team_id: int, **params) | |
| Mlb.get_schedule(date: str = None, start_date: str = None, end_date: str = None, sport_id: int = 1, team_id: int = None, **params) | |
| Mlb.get_scheduled_games_by_date(date: str = None, start_date: str = None, end_date: str = None, sport_id: int = 1, **params) | |
| Mlb.get_schedule(date: str = None, start_date: str = None, end_date: str = None, sport_id: int = 1, team_id: int = None, **params) | |
| Mlb.get_scheduled_games_by_date(date: str = None, start_date: str = None, end_date: str = None, sport_id: int = 1, **params) |
| ## Supported endpoints | ||
|
|
||
| The async surface is intentionally smaller than the synchronous `Mlb` surface | ||
| while 1.1 support is being expanded. The currently supported awaitable endpoint | ||
| methods on `release/1.1.0` are: | ||
|
|
||
| ```text | ||
| get_team(...) | ||
| get_teams(...) | ||
| get_person(...) | ||
| get_people(...) | ||
| get_schedule(...) | ||
| ``` |
There was a problem hiding this comment.
Inconsistency: this "Supported endpoints" list states the only currently-awaitable methods on release/1.1.0 are get_team, get_teams, get_person, get_people, and get_schedule. But docs/stats.md (added in this same PR) documents and demonstrates all four stats methods as awaitable on AsyncMlb, e.g.:
python-mlb-statsapi/docs/stats.md
Lines 83 to 106 in dbe46e7
async def main():
async with AsyncMlb() as mlb:
stats = await mlb.get_player_stats(...)(similarly for get_team_stats, get_stats, and get_players_stats_for_game in the same file). README.md's new "Stats" section also promises stats.md examples "using both Mlb and AsyncMlb". One of these two new docs is wrong about the current async surface — they should be reconciled.
python-mlb-statsapi/docs/async.md
Lines 91 to 105 in dbe46e7
| ## Common Methods | ||
|
|
||
| ### Pull Request Guidelines | ||
| ### Players | ||
|
|
||
| - Run offline tests before submitting a PR | ||
| - Use the [PR template](.github/pull_request_template.md) when creating your pull request | ||
| - Follow the branch naming convention: | ||
| - `feat/` - New features | ||
| - `fix/` - Bug fixes | ||
| - `docs/` - Documentation updates | ||
| - `refactor/` - Code improvements | ||
|
|
||
| ### Reporting Issues | ||
|
|
||
| Found a bug or have a feature request? Please [open an issue](https://github.com/zero-sum-seattle/python-mlb-statsapi/issues/new) with: | ||
|
|
||
| - A clear description of the problem or feature | ||
| - Steps to reproduce (for bugs) | ||
| - Expected vs actual behavior | ||
| - Python version and package version | ||
|
|
||
|
|
||
| ## Examples | ||
|
|
||
| Let's show some examples of getting stat objects from the API. What is baseball without stats, right? | ||
|
|
||
| ### Player Stats | ||
| Get the Id(s) of the players you want stats for and set stat types and groups. | ||
| ```python | ||
| >>> mlb = mlbstatsapi.Mlb() | ||
| >>> player_id = mlb.get_people_id("Ty France")[0] | ||
| >>> stats = ['season', 'career'] | ||
| >>> groups = ['hitting', 'pitching'] | ||
| >>> params = {'season': 2022} | ||
| player = mlb.get_person(664034) | ||
| players = mlb.get_people() | ||
| player_ids = mlb.get_people_id("Ty France") | ||
| ``` | ||
|
|
||
| Use player id with stat types and groups to return a stats dictionary | ||
| ```python | ||
| >>> stat_dict = mlb.get_player_stats(player_id, stats=stats, groups=groups, **params) | ||
| >>> season_hitting_stat = stat_dict['hitting']['season'] | ||
| >>> career_pitching_stat = stat_dict['pitching']['career'] | ||
| ``` | ||
| ### Teams | ||
|
|
||
| Print season hitting stats using Pydantic's `model_dump()` | ||
| ```python | ||
| >>> for split in season_hitting_stat.splits: | ||
| ... print(split.stat.model_dump(exclude_none=True)) | ||
| {'games_played': 140, 'groundouts': 163, 'airouts': 148, 'runs': 65, 'doubles': 27, ...} | ||
| team = mlb.get_team(136) | ||
| teams = mlb.get_teams() | ||
| team_ids = mlb.get_team_id("Seattle Mariners") | ||
| ``` | ||
|
|
||
| Or access individual fields directly | ||
| ```python | ||
| >>> for split in season_hitting_stat.splits: | ||
| ... print(f"Games: {split.stat.games_played}") | ||
| ... print(f"Home Runs: {split.stat.home_runs}") | ||
| ... print(f"Batting Avg: {split.stat.avg}") | ||
| Games: 140 | ||
| Home Runs: 20 | ||
| Batting Avg: .274 | ||
| ``` | ||
| ### Stats | ||
|
|
||
| ### Team Stats | ||
| Get the Team Id(s) | ||
| ```python | ||
| >>> mlb = mlbstatsapi.Mlb() | ||
| >>> team_id = mlb.get_team_id('Seattle Mariners')[0] | ||
| ``` | ||
| The stats API has several entry points and returns a nested `stats[group][type]` structure. See the dedicated [Stats Guide](docs/stats.md) for `get_player_stats()`, `get_team_stats()`, `get_stats()`, and `get_players_stats_for_game()` examples using both `Mlb` and `AsyncMlb`. | ||
|
|
||
| Set the stat types and groups | ||
| ```python | ||
| >>> stats = ['season', 'seasonAdvanced'] | ||
| >>> groups = ['hitting'] | ||
| >>> params = {'season': 2022} | ||
| ``` | ||
|
|
||
| Use team id and the stat types and groups to return season hitting stats | ||
| ```python | ||
| >>> stats = mlb.get_team_stats(team_id, stats=stats, groups=groups, **params) | ||
| >>> season_hitting = stats['hitting']['season'] | ||
| >>> advanced_hitting = stats['hitting']['seasonAdvanced'] | ||
| ``` | ||
|
|
||
| Print stats as JSON | ||
| ```python | ||
| >>> for split in season_hitting.splits: | ||
| ... print(split.stat.model_dump_json(indent=2, exclude_none=True)) | ||
| { | ||
| "games_played": 162, | ||
| "groundouts": 1273, | ||
| "runs": 690, | ||
| "doubles": 229, | ||
| ... | ||
| } | ||
| ``` | ||
| ### Schedule | ||
|
|
||
| ### Expected Stats | ||
| ```python | ||
| >>> player_id = mlb.get_people_id('Ty France')[0] | ||
| >>> stats = ['expectedStatistics'] | ||
| >>> group = ['hitting'] | ||
| >>> params = {'season': 2022} | ||
|
|
||
| >>> stats = mlb.get_player_stats(player_id, stats=stats, groups=group, **params) | ||
| >>> expected = stats['hitting']['expectedStatistics'] | ||
| >>> for split in expected.splits: | ||
| ... print(f"Expected AVG: {split.stat.avg}") | ||
| ... print(f"Expected SLG: {split.stat.slg}") | ||
| Expected AVG: .259 | ||
| Expected SLG: .394 | ||
| schedule = mlb.get_schedule(date="2022-10-13") | ||
| ``` |
There was a problem hiding this comment.
Bug: the Players, Teams, and Schedule snippets under "Common Methods" call mlb.get_person(...), mlb.get_team(...), mlb.get_schedule(...) etc. with no preceding import or client construction (no from mlbstatsapi import Mlb, no with Mlb() as mlb:). Copy-pasted standalone, each raises NameError: name 'mlb' is not defined.
Every other snippet in this rewritten README (Quick Start, HTTP and Error Behavior, Working with Models) includes the setup. docs/examples.md, added in this same PR, follows the same convention for its equivalent sections. These three blocks are the only outliers.
Lines 172 to 200 in dbe46e7
Suggested fix: add a short setup line (or note that these assume the mlb client from Quick Start) to each of the three snippets.
6f6c2e4 to
509567c
Compare
Wraps the reusable async logic in a plain function so it stays valid Python without prescribing a main() entry point; the asyncio.run() wrapper is now clearly marked as just one way to invoke it. Co-authored-by: Matthew Spah <2068393+Mattsface@users.noreply.github.com>
Wrapping every advanced async example in a main()/asyncio.run() entry point isn't practical for readers integrating into an existing app. Show a script entry point alongside patterns for an already-running event loop, FastAPI, and interactive/notebook use with top-level await. Co-authored-by: Matthew Spah <2068393+Mattsface@users.noreply.github.com>
|
|
||
| `AsyncMlb` mirrors the endpoint surface exposed by `Mlb`. Its endpoint methods | ||
| are asynchronous and return the same parsed Pydantic model types while following | ||
| the same public HTTP/error behavior as their synchronous counterparts. | ||
|
|
There was a problem hiding this comment.
Conflicting claims about async endpoint coverage. This states AsyncMlb unconditionally mirrors the full endpoint surface of Mlb. But README.md's new "Sync or Async?" section (added in this same PR) hedges the opposite way: "Where an async endpoint is supported...", and separately calls out get_schedule is available on both Mlb and AsyncMlb as a specific fact — which only makes sense to single out if async coverage is not universal.
One of these two framings is inaccurate. Worth reconciling before merge, especially since the PR description itself calls out "async endpoint coverage is still being finalized" as the main risk of this change.
|
@claude another pass please |
|
Claude finished @Mattsface's task in 3m 5s —— View job Another pass — done
What I found and fixedI verified the actual The remaining outstanding review comment (from 22:49 UTC) flagged that
Since the ground truth (confirmed against source) is that
While doing a full pass over
I also re-verified every Python code block across |
- README now states plainly that AsyncMlb mirrors the full Mlb endpoint surface, matching docs/async.md and docs/public-api.md, instead of hedged wording that implied partial coverage. - Drop the README callout singling out get_schedule as available on both clients, since that only made sense under partial coverage. - Fix get_awards, get_season, get_seasons, and get_people_id signatures in docs/methods.md to match mlbstatsapi/mlb_api.py. Co-authored-by: Matthew Spah <2068393+Mattsface@users.noreply.github.com>
1 participant
Why
The README has grown into a mix of quick-start documentation, transport reference material, migration guidance, endpoint documentation, and release history.
For v1.1.0, the goal is to make the README easier to scan for new users while preserving the deeper technical documentation in dedicated pages.
This also adds clear documentation for the new async client without turning the README into another full API reference.
Tracks #306.
What
docs/async.mdfor detailedAsyncMlbusagedocs/examples.mdfor longer usage examplesdocs/methods.mdTests
This PR contains documentation changes only.
The documentation was reviewed against the current
release/1.1.0API and async contract. Code examples and documented method names were checked against the current public API documentation.Normal CI should still run before merge.
Risk and impact
Risk: Minimal
There are no production code or public API changes in this PR.
The main risk is documentation becoming inaccurate or linking users to the wrong guidance, particularly while async endpoint coverage is still being finalized.
If something does go wrong, the impact should be limited to confusing or incorrect documentation. It would not change runtime behavior for existing users.