Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing 'qualifiers' key in event_factory kwargs #209

Closed

Conversation

UnravelSports
Copy link
Contributor

Hi,

I was trying to load some Opta data and got the following error:

TypeError: TakeOnEvent.__init__() missing 1 required positional argument: 'qualifiers'

I checked and it seemed to only occur at TakeOnEvents (for this particular game of data I was testing).

I simply added these two lines to add a qualifiers key to kwargs if it doesn't exist.

 if 'qualifiers' not in kwargs:
        kwargs['qualifiers'] = None

It seems consistent with the lines above in the create_event function where we add "freeze_frame" and/or "related_event_ids" if they don't exist. Like so:

def create_event(event_cls: Type[T], **kwargs) -> T:
    """
    Do the actual construction of an event.

    This method does a couple of things:
    1. Fill in some arguments when not passed
    2. Pass only arguments that are accepted by the `event_cls`.
       This is required in cases an EventFactory initialized less rich
       Events than data is passed for. E.g. `expected_goal` is passed
       to a regular `ShotEvent`.
       Normally this would break because of an 'Unexpected argument' exception,
       but we filter those arguments out.
    """
    extra_kwargs = {"state": {}}
    if "related_event_ids" not in kwargs:
        extra_kwargs["related_event_ids"] = []

    if "freeze_frame" not in kwargs:
        kwargs["freeze_frame"] = None

   etc. etc.

@koenvo
Copy link
Contributor

koenvo commented Aug 23, 2023

@UnravelSports can you add TakeOn event to the Opta test to make sure it works with real data?

@koenvo koenvo added this to the 3.13 milestone Sep 20, 2023
@koenvo
Copy link
Contributor

koenvo commented Nov 16, 2023

I'm trying to figure out why/where this breaks and I can't seem to find it. The build_take_on call from the Opta deserializer does include qualifiers

@JanVanHaaren
Copy link
Collaborator

I'm trying to figure out why/where this breaks and I can't seem to find it. The build_take_on call from the Opta deserializer does include qualifiers

I suspect the positional argument qualifiers should be provided before the keyword arguments.

event = self.event_factory.build_take_on(
	**take_on_event_kwargs,
	**generic_event_kwargs,
	qualifiers=None,
)
event = self.event_factory.build_take_on(
	qualifiers=None,
	**take_on_event_kwargs,
	**generic_event_kwargs,
)

@koenvo
Copy link
Contributor

koenvo commented Nov 16, 2023

I'm trying to figure out why/where this breaks and I can't seem to find it. The build_take_on call from the Opta deserializer does include qualifiers

I suspect the positional argument qualifiers should be provided before the keyword arguments.

event = self.event_factory.build_take_on(
	**take_on_event_kwargs,
	**generic_event_kwargs,
	qualifiers=None,
)
event = self.event_factory.build_take_on(
	qualifiers=None,
	**take_on_event_kwargs,
	**generic_event_kwargs,
)

Noticed the same. But can't understand why this could make a difference. I believe in both cases they are all keyword arguments.

>>> a = {}
>>> b = {}
>>> c = dict(*a, *b, piet=1)
>>> c
{'piet': 1}
>>> d = dict(piet=1, *a, *b)
>>> d
{'piet': 1}

@JanVanHaaren
Copy link
Collaborator

@koenvo I believe this pull request is no longer relevant. While this issue indeed occurs in the most recent release, it appears to have been addressed already in the master branch.

@koenvo koenvo closed this Dec 26, 2023
@koenvo koenvo removed this from the 3.14 - Pi milestone Dec 29, 2023
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.

3 participants