Wrong understanding of the LeastRecentActors
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
Zeitgeist Framework |
Fix Released
|
Low
|
Seif Lotfy |
Bug Description
In an attempt to work on bug #641968 I discovered that we some of us defer on the understanding of LeastRecentActor
The documentation stated that LeastRecentActor = enum_factory(("The first event of each different actor"))
Let's assume we have sequential events. (The actors are defined by numbers)
2, 1, 3, 2, 1, 4
So we have 4 different actors (1,2,3,4) and we want to sort them by least recent.
the least recent is not 2 or 1 since they are used again at the end. the least recent is 3
This means LeastRecentActors should return the latest actors sorted ASC:
3, 2, 1, 4
and not
2, 1, 3, 4
When we look at LeastRecentSubjects = enum_factory(("One event for each subject only, "
"ordered with oldest events first"))
My understanding according to Siegfried is:
<seif_> RainCT,
<seif_> LeastRecentSubjects = enum_factory(("One event for each subject only, "
<seif_> "ordered with oldest events first")
<seif_> so i f i have
<seif_> the subject
<seif_> 1, 2, 1, 3, 4
<seif_> what do i get returned
<seif_> 1, 2, 3, 4
<seif_> or
<seif_> 2, 1, 3, 4
<seif_> ?
<RainCT> seif_: the later
<RainCT> for each subject you only look at the most recent one
<seif_> ok then we should do the same for the actors :)
<RainCT> Yes. Isn't it like this already?
<seif_> no
In that case if we follow this convention I can update the doc strings and already have the bug fix for both this bug and #641968
Related branches
- Siegfried Gevatter: Approve
-
Diff: 129 lines (+44/-20)3 files modified_zeitgeist/engine/main.py (+3/-2)
test/engine-test.py (+38/-16)
zeitgeist/datamodel.py (+3/-2)
Changed in zeitgeist: | |
assignee: | nobody → Seif Lotfy (seif) |
description: | updated |
Changed in zeitgeist: | |
importance: | Undecided → Low |
status: | New → Triaged |
milestone: | none → 0.6 |
Changed in zeitgeist: | |
status: | Triaged → In Progress |
Changed in zeitgeist: | |
status: | In Progress → Fix Committed |
milestone: | 0.6 → 0.5.2 |
Changed in zeitgeist: | |
milestone: | 0.5.2 → 0.6 |
milestone: | 0.6 → 0.5.2 |
Changed in zeitgeist: | |
status: | Fix Committed → Fix Released |
Taking the LeastRecentSubjects example. I'm going to assume you have time=0 on the left and time>0 on the right: 1, 2, 1, 3, 4.
Then "One event for each subject only, ordered with oldest events first" would definitely mean: 1, 2, 3, 4. If we are sorting like RainCT is proposing then the docstring should probably be "The most recent event for each subject, ordered with the oldest events first" or something like that.
That said I am actually not sure at all what the right API is. WHich use cases do we have?