Improve handling when specific events are missing.#397
Improve handling when specific events are missing.#397JanEricNitschke wants to merge 3 commits intopnxenopoulos:mainfrom
Conversation
…this event is missing most of the time
There was a problem hiding this comment.
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
|
Also improved the args handling for Closes: #395 |
1fce3d1 to
ad25725
Compare
| "rounds", | ||
| ]: | ||
| try: | ||
| parsed_df: pl.DataFrame = getattr(self, df_name) |
bkushigian
left a comment
There was a problem hiding this comment.
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.
| from loguru import logger | ||
|
|
||
|
|
||
| def get_event_from_parsed_events( |
There was a problem hiding this comment.
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.
|
Is it on the cards for this to get merged? Bumped into an issue today that I think this will solve. |
Closes: #396