Improvements and tweaks#649
Open
sreckoskocilic wants to merge 6 commits intoswar:masterfrom
Open
Conversation
- refactor get_parameters update {k:v} anti-pattern
- improve find_player_by_id with single dict get() call
- find_players search speedup
…mmit - status check in raise_exception_on_error raises on HTTP 4xx/5xx before checking json validity (previously a 429 or 500 would silently pass)
…ecks) - added strip_accents for find_teams - ImportError should not be silenced as it could indicate a broken module (parser with syntax error)
swar
reviewed
Mar 10, 2026
| endpoint_parser = get_parser_for_endpoint(self._endpoint, raw_data) | ||
| for name, dataset in endpoint_parser.get_data_sets().items(): | ||
| data[name] = self._build_rows(dataset["headers"], dataset["data"]) | ||
| except (KeyError, ImportError): |
Owner
There was a problem hiding this comment.
Why remove the import error here? To be honest, I'm not sure why it's even catching it here in the first place
Author
There was a problem hiding this comment.
If the import of get_parser_for_endpoint fails, that's a critical issue that should raise an exception, not silently
continue with an empty result
Owner
|
No major complaints from my initial pass. I need to do a deeper dive here and then push this one through. All the changes you suggested make sense |
swar
reviewed
Mar 10, 2026
| if raise_exception_on_error and not data.valid_json(): | ||
| raise Exception("InvalidResponse: Response is not in a valid JSON format.") | ||
| if raise_exception_on_error: | ||
| if status_code is not None and status_code >= 400: |
Owner
There was a problem hiding this comment.
Why the additional breakout of the exception?
Author
There was a problem hiding this comment.
The code now checks the HTTP status code first (any >= 400) raising an Exception with the status code, before
validating JSON. This ensures HTTP errors are properly caught.
…ad of mutating class-level STATS_HEADERS) - fix debug cache bypassing raise_exception_on_error (set status_code=200 when loading from file) - fix silent KeyError swallowing in v3 parser dispatch (get_parser_for_endpoint returns None instead of raising) - fix get_data_sets KeyError when neither resultSets nor resultSet present (use .get() with guard) - fix Makefile referencing dead tools (isort/black/flake8/pylint → ruff) - fix live http.py using wrong variable name STATS_HEADERS → LIVE_HEADERS - add type hints to public API (players.py, teams.py, http.py, live _base.py) - extract _get_legacy_results helper to DRY repeated resultSets/resultSet lookup - improve _base.py: mutable default → None, .get() patterns, ImportError, get_available_data returns list - add 15 regression tests for bug fixes
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I have analyzed my demo app along with external libraries used. It is using nba_api lib to keep track on recent games and player stats. Probably it would be good to make a revision on the fixes provided in this PR, especially the one reducing redundant json.loads by implementing dict_cache. Notable is reducing get_parameters() calls of get_dict() from 3 to 1 and find_players() compiles re only one per search now.
Changes summary
calls don't re-parse. Fixed a bug where passing a referer would mutate the shared class-level
STATS_HEADERS dict permanently. Debug cache path was silently skipping raise_exception_on_error checks —
fixed by setting status_code=200 on cache hits. Added type annotations to NBAResponse, NBAHTTP, and
send_api_request().
pulled it into _get_legacy_results(). A bare except KeyError: pass was eating errors from v3 parsers, so
parser bugs would just silently return empty data. get_data_sets() would crash with a KeyError if neither
resultSets nor resultSet existed in the response.
pre-built dicts instead of scanning the full list. Regex compilation is LRU-cached. Name searches strip
accents, so Jokić and Jokic both match. Added type hints to all public functions.
instances — changed to None. Replaced if "key" in dict checks with .get() and defaults.
get_available_data() was returning a dict_keys view instead of a list. Missing pandas now raises
ImportError instead of a bare Exception.
unregistered endpoints. Now returns None, which means actual parser bugs (also KeyErrors) stop getting
silently caught by the same handler.
the live HTTP module — it was pointing at CDN headers, not stats headers.
installed. Replaced with ruff. Added make lint-fix.
helper, and the missing key guards.
Test plan