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

events_from_type could return ordered dictionaries #56

Closed
absolutelysimon opened this issue Jan 8, 2019 · 5 comments
Closed

events_from_type could return ordered dictionaries #56

absolutelysimon opened this issue Jan 8, 2019 · 5 comments

Comments

@absolutelysimon
Copy link

All events include a timestamp. In my limited testing, the events come out sorted anyways. If we order the events and put them into a dictionary, subsequent operations on the resulting ordered dictionary will be much faster, some will be O(1) rather than O(n).

This would be an improvement, in my opinion. If there is concern about backwards compatibility (which there should be) we could simply have it as an optional flag such as .events_from_type(event, ordered_dict = False)

I'd be happy to take a look at implementing this if there is interest/agreement.

@ramonsaraiva
Copy link
Owner

In that case, events_from_type(..) would return an OrderedDict instead? What would be the keys and values?

@absolutelysimon
Copy link
Author

Correct. The keys would be the timestamps and the values would the events themselves. Then if I have an event, say LogPlayerMakeGroggy and I want to find a kill event within sixty seconds after that I can get a slice of the dict in O(1) time.

@ramonsaraiva
Copy link
Owner

It makes sense, but at the same time, it would be weird to iterate over the list of LogPlayerMakeGroggy events, especially when you might end up having two events in the same timestamp.

If the events are already sorted when you do events_from_type by timestamp, the cost to retrieve an event in a specific timestamp is probably log n if you use something like bisect https://docs.python.org/3/library/bisect.html

@ramonsaraiva
Copy link
Owner

I don't mind having an additional functionality to arrange the events by timestamp as you proposed. Since it would need to be done when fetching a specific type of events, If you want to, feel free to add something like arrange_by_timestamp or something similar. Then that could accept any event iterable and arrange it as you said.

@absolutelysimon
Copy link
Author

You're totally right about the collisions, there are thousands per match. I do think the arrange_by_timestamp would be helpful. I'll take a look at it!

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

No branches or pull requests

2 participants