Skip to content

Improve handling when specific events are missing.#397

Open
JanEricNitschke wants to merge 3 commits intopnxenopoulos:mainfrom
JanEricNitschke:handle-missing-attributes
Open

Improve handling when specific events are missing.#397
JanEricNitschke wants to merge 3 commits intopnxenopoulos:mainfrom
JanEricNitschke:handle-missing-attributes

Conversation

@JanEricNitschke
Copy link
Copy Markdown
Collaborator

Closes: #396

@JanEricNitschke JanEricNitschke requested a review from Copilot April 11, 2025 06:11
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

awpy/demo.py:540

  • Changing the events_to_parse parameter from optional to required may break existing functionality if callers expect its default value. Consider either retaining the default parameter value or updating all call sites accordingly.
events_to_parse: list[str],

awpy/parsers/utils.py:4

  • The new dependency on loguru requires ensuring that it is documented in the project's dependency list to avoid runtime errors on environments without it.
from loguru import logger

@JanEricNitschke
Copy link
Copy Markdown
Collaborator Author

Also improved the args handling for parse a bit.

Closes: #395

@JanEricNitschke JanEricNitschke changed the title Skip footsteps if respective event is missing instead of crashing as this event is missing most of the time Improve handling when specific events are missing. Apr 11, 2025
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comment thread awpy/demo.py
Comment thread tests/test_cli.py Outdated
@JanEricNitschke JanEricNitschke force-pushed the handle-missing-attributes branch from 1fce3d1 to ad25725 Compare April 11, 2025 08:55
Comment thread awpy/demo.py
"rounds",
]:
try:
parsed_df: pl.DataFrame = getattr(self, df_name)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah this is a lot cleaner IMO

Copy link
Copy Markdown

@bkushigian bkushigian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I have one comment about the type of utils::get_event_from_parsed_events but I'm not convinced, just putting it up there for consideration.

Comment thread awpy/parsers/utils.py
from loguru import logger


def get_event_from_parsed_events(
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would argue this should be Optional[pl.DataFrame] so that the type is large enough to have all possible outcomes (no list + empty list). But this might not be an issue in practice and would require some extra work elsewhere in the code. Plus, optionals are only good if their usage is enforced by the type system, so this is probably better.

@n1c
Copy link
Copy Markdown
Contributor

n1c commented May 29, 2025

Is it on the cards for this to get merged? Bumped into an issue today that I think this will solve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CLI awpy parse crashes w/ KeyError: "player_sound" on default settings for some demos

4 participants