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

Add support for matches #112

Open
Olen opened this issue Apr 25, 2024 · 15 comments · May be fixed by elliot-100/Spond-classes#148
Open

Add support for matches #112

Olen opened this issue Apr 25, 2024 · 15 comments · May be fixed by elliot-100/Spond-classes#148
Labels
enhancement New feature or request

Comments

@Olen
Copy link
Owner

Olen commented Apr 25, 2024

Should probably refactor event to a class first, so match can be a subclass of event

@Olen
Copy link
Owner Author

Olen commented Apr 25, 2024

New parameters for matches:

 'matchEvent': True,
 'matchInfo': {
               'opponentName': 'Hasle-Løren Hvit',
               'opponentScore': 6,
               'scoresFinal': True,
               'scoresPublic': True,
               'scoresSet': True,
               'scoresSetEver': True,
               'teamName': 'Grüner lilla',
               'teamScore': 11,
               'type': 'AWAY'
},

type can be HOME and AWAY

@elliot-100 elliot-100 added the enhancement New feature or request label Apr 25, 2024
@elliot-100
Copy link
Collaborator

Can't help with this one -- at least not with real-world examples -- as Matches aren't relevant/available to my clubs (documentation on which sports are supported).

@Olen
Copy link
Owner Author

Olen commented Apr 25, 2024

They are just regular events, but with a few more fields (noted in the first comment)

@Bart274
Copy link

Bart274 commented Dec 24, 2024

This is something that would be really useful. Any progress on this?

@elliot-100
Copy link
Collaborator

elliot-100 commented Dec 25, 2024

Olen raised a PR against my separate Spond-classes package: elliot-100/Spond-classes#148

I should be able to add it in the next few weeks.

@Bart274
Copy link

Bart274 commented Dec 25, 2024

Nice. That would really help us a lot 😊

@elliot-100
Copy link
Collaborator

elliot-100 commented Dec 28, 2024

@Bart274 can you have a look at the PR elliot-100/Spond-classes#148 please? I don't think I can tag you from there.

@Bart274
Copy link

Bart274 commented Dec 28, 2024

@elliot-100 it looks good. But I don’t see why that PR is blocking any development for this issue here? I use the spond package without the spond-classes at this moment.

@elliot-100
Copy link
Collaborator

@Bart274 given:

They are just regular events, but with a few more fields

I'm not clear what you want spond to do? Could you give an example?

@Bart274
Copy link

Bart274 commented Dec 28, 2024

@elliot-100 well, the matchinfo fields are missing in the event template:

_EVENT_TEMPLATE: JSONDict = {

Since these fields are missing, even when you retrieve an existing event from Spond that contains the data, whenever you call the method to update existing events, it will remove the data from the event in the function as it’s keys are missing in the template.
for key in base_event:

So we can’t update the matchinfo of events through this API.

@elliot-100
Copy link
Collaborator

elliot-100 commented Dec 28, 2024

OK, can you raise a separate bug issue instead, please, as this doesn't really relate to adding a class interface for events.

(although I can see that the list of additional params might give that impression)

@Bart274
Copy link

Bart274 commented Dec 28, 2024

@elliot-100 but isn’t this the point of this ticket? The issue is to add support for matches… 😄
We can retrieve the matchinfo from existing events just fine, but we can’t update them. What else is this ticket for?

@elliot-100
Copy link
Collaborator

elliot-100 commented Dec 28, 2024

The package does support matches in general, like any other event.

@Olen, who is the author of the package, created this issue and has decided that at this time, what they want is best implemented as an addition to my existing class interface. And so has raised a linked closing PR there. That is their perogative.

It was not clear from your initial supporting comment in the conversation that you were expecting something else. So I (the other main contributor to this package) have just spent some development time on this during holidays, partly in order to help you.

You could now help us and other users and contributors by raising a separate issue, so we, and you, can track it more easily. For example you might wish to raise a PR yourself against that in order to contribute further.

@Bart274
Copy link

Bart274 commented Dec 29, 2024

@elliot-100 matches are supported to retrieve information. However, since the matchevent & matchinfo keys are missing in the event template, you can't use the update_event method to update that information.
I have added that information in the event template in this PR: #169

@Olen
Copy link
Owner Author

Olen commented Dec 29, 2024

My main point was that I did not want to do any major changes here before the classes PR was merged, as I really think that is the way forward.

I have not followed the development of that branch tightly, so I am not sure how close to ready that branch is.
But if it feals mature enough, I am more than happy to merge it here and create a breaking release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants