Adds MVP info #253
Adds MVP info #253
Conversation
# Conflicts: # awpy/analytics/stats.py # awpy/parser/parse_demo.go # awpy/types.py
There was a problem hiding this comment.
Hi, thanks for your contribution.
Looks very good to me already. I just added some small comments.
I also see that the python part is failing the linting.
We have established a fairly thorough formatting/linting/testing process using black, ruff, pyright, pylint and pytest.
https://github.com/pnxenopoulos/awpy/blob/main/.github/workflows/build.yml
Some of these checks you can very easily do locally via pre-commit:
https://github.com/pnxenopoulos/awpy/blob/main/.pre-commit-config.yaml
https://pre-commit.com/
For the others you would have to manually install pyright and pylint or use the CI to check if the pass.
It would be great if you could adjust your code so that it passes all of the linting steps.
There was a problem hiding this comment.
Really great work on this one! I appreciate the edits on the notebooks as well. I only have a few small comments that need to be addressed, then we can merge this one into main:
- in the
GameRoundstruct, changeMVPName,MVPSteamID, andMVPReasonto be pointers by using* - in the awpy types file, use lowercase casing (e.g.
mvpName) for the new columns
with those changes, this new feature will be complete
awpy/analytics/stats.py
Outdated
| player_statistics: dict[str, PlayerStatistics], | ||
| round_statistics: RoundStatistics, | ||
| ) -> None: | ||
| if "mvpName" in game_round: |
There was a problem hiding this comment.
That check should be unnecessary since we per type definition a "GameRound" should always have that key.
But it probably does make sense to leave this in to allow the stat extraction of old json files that were produced with a previous version.
awpy/analytics/stats.py
Outdated
| round_statistics: RoundStatistics, | ||
| ) -> None: | ||
| if "mvpName" in game_round: | ||
| player_key = ( |
There was a problem hiding this comment.
This could probably make use of _get_actor_key
|
What is with the |
|
Tests fail because the manual game_rounds do not include that key: Line 182 in 3945c2e It might actually make sense to keep that check to not break on old parsed jsons. |
No description provided.