#10686 closed defect (fixed)
Strange sorting after recent updates
Reported by: | Chris Nelson | Owned by: | Chris Nelson |
---|---|---|---|
Priority: | high | Component: | TracJsGanttPlugin |
Severity: | major | Keywords: | |
Cc: | falkb | Trac Release: | 0.12 |
Description (last modified by )
As falkb reported starting at ticket:9648#comment:35, sorting of some datasets after got weird between r12213 and r12330.
(This got to be more than a quick consultation or fix so it deserves its own ticket.)
Attachments (4)
Change History (28)
comment:1 Changed 12 years ago by
Status: | new → assigned |
---|
comment:2 Changed 12 years ago by
The original chart is drawn with
[[TracJSGanttChart(milestone=foo,colorBy=status,userMap=0,format=week,schedule=alap,doResourceLeveling=0)]]
I'm trying to reproduce it with:
[[TracJSGanttChart(keywords=TH9648,format=week,doResourceLeveling=0,schedule=alap)]]
Is it possible that the selection by milestone vs. keywords matters?
comment:3 follow-up: 7 Changed 12 years ago by
My durations don't match the examples of good and bad behavior. 2690 is 5 days in the screen shots but 1 day on my chart.
Perhaps we're using different calendars. I have the TracPMConnector in Team Calendar disabled and the SimpleCalendar in TracJSGantt enabled.
comment:4 Changed 12 years ago by
[[TracJSGanttChart(milestone=th9648,format=week,doResourceLeveling=0,schedule=alap,order=wbs)]]
displays correctly for me (though I can't explain why; that's the default at r12330).
comment:5 Changed 12 years ago by
What version of Python are you using? I thought that some core sorting features changed in a late version. I'm using 2.7.3.
comment:6 Changed 12 years ago by
0.12.2 uses 2.5, but I'm also seeing the problems with 1.0.0 which uses 2.7
Changed 12 years ago by
comment:7 follow-ups: 9 13 Changed 12 years ago by
Replying to ChrisNelson:
My durations don't match the examples of good and bad behavior. 2690 is 5 days in the screen shots but 1 day on my chart.
This is simply because the worked hours exceeded the estimated hours. If worked < estimated, the bar length shows estimated hours; otherwise if worked > estimated, the bar length shows worked hours. This is a mixup that I've criticized in #8786 already. I reckon you must also set the values for estimated and worked hours to get similar display behaviour. I've attached it in attachment:hours.csv.
Remember I've set
[TracPM] fields.estimate = estimatedhours fields.worked = totalhours
Perhaps we're using different calendars. I have the TracPMConnector in Team Calendar disabled and the SimpleCalendar in TracJSGantt enabled.
Actually, I've never cared about the calender stuff, actually I don't know what it means at all. I don't have installed any calender plugins either. Anyway I searched the trac.ini for occurences of pattern 'alend' and just found this:
tracjsgantt.tracpm.calendarscheduler = enabled tracjsgantt.tracpm.simplecalendar = enabled
comment:8 follow-up: 10 Changed 12 years ago by
Another idea: We should check if we use the same plugin settings. I have
[x] TracJSGanttChart [x] TracJSGanttSupport - [ ] ProjectSorter [x] ResourceScheduler [x] SimpleCalendar [ ] SimpleSorter [x] TracPM
Which setting should I use? Why are there 2 sorters?
comment:9 follow-up: 10 Changed 12 years ago by
Replying to falkb:
Replying to ChrisNelson:
My durations don't match the examples of good and bad behavior. 2690 is 5 days in the screen shots but 1 day on my chart.
This is simply because the worked hours exceeded the estimated hours.
That makes sense. Thanks.
...
Perhaps we're using different calendars. I have the TracPMConnector in Team Calendar disabled and the SimpleCalendar in TracJSGantt enabled.
Actually, I've never cared about the calender stuff, actually I don't know what it means at all. I don't have installed any calender plugins either.
The scheduler in TracPM defers to a calendar to determine how much work is possible on a day. The default, built-in calendar basically says "we don't work on weekends" but you can replace that calendar if you have special needs. I put a TracPMConnector into TeamCalendar so that individual vacation days, etc. are considered when scheduling.
Anyway I searched the trac.ini for occurences of pattern 'alend' and just found this:
tracjsgantt.tracpm.calendarscheduler = enabled tracjsgantt.tracpm.simplecalendar = enabled
OK. "SimpleCalendar" is the week-days-only I mentioned above.
comment:10 Changed 12 years ago by
Replying to anonymous:
... but you can replace that calendar if you have special needs. I put a TracPMConnector into TeamCalendar so that individual vacation days, etc. are considered when scheduling.
I see. Sounds like a cool feature.
comment:10 Changed 12 years ago by
Replying to falkb:
Another idea: We should check if we use the same plugin settings. I have
[x] TracJSGanttChart [x] TracJSGanttSupport - [ ] ProjectSorter [x] ResourceScheduler [x] SimpleCalendar [ ] SimpleSorter [x] TracPMWhich setting should I use? Why are there 2 sorters?
There are two sorters so you can pick the one that you prefer and so that I make sure I'm not a victim of the zero-one-many problem. These sorters are for the schedler, not the Gantt display, they control which task to pick next when more than one is eligible (has all predecessors complete, etc.).
SimpleSorter
compares two tasks based on the enum values for their priorities (e.g., a Major ticket comes before a Minor, etc.).
ProjectSorter
(probably a bad name) computes an "effective" priority for a task based on its ancestors. If A has a priority (enum) of 1 and its children B and C have priorities of 2 and 3, respectively, the B has an effective priority of [1 2] and C has [1 3]. Then if D (priority 2) has a child E (1), E's effective priority is [2 1] so even though its assigned priority is higher than B or C, it will get scheduled later because its parent is lower priority.
If you don't select a sorter, the SimpleSorter is used by default.
I have the Project Sorter enabled but since all your task priorities are the same, you're not doing resource leveling, and the problem is not in the scheduling (the tasks have the same begin and end date in both charts), I don't think that is the problem.
comment:11 Changed 12 years ago by
Can you please try this patch:
-
tracjsgantt/tracjsgantt.py
a b All other macro arguments are treated as TracQuery specifica 464 464 name = "#%d:%s (%s %s)" % \ 465 465 (ticket['id'], ticket['summary'], 466 466 ticket['status'], ticket['type']) 467 self.env.log.debug('Next task is %s, wbs %s' % 468 (ticket['id'], ticket['wbs'])) 467 469 task += 't = new JSGantt.TaskItem(%s,%d,"%s",' % \ 468 470 (self.GanttID, ticket['id'], javascript_quote(name)) 469 471 … … All other macro arguments are treated as TracQuery specifica 554 556 555 557 # Get all the sort fields 556 558 sortFields = options['order'].split('|') 559 self.env.log.debug('sortFields:%s' % sortFields) 557 560 558 561 # If sorting by milestone, force milestone type tickets to the 559 562 # end before any other sort. The stability of the other sorts
and send me the results after displaying your problem Gantt?
Changed 12 years ago by
Attachment: | trac_debug1.txt added |
---|
Changed 12 years ago by
Attachment: | TH10686-with-hours.png added |
---|
My Gantt with estimated and total hours added to tasks
comment:13 Changed 12 years ago by
Replying to falkb:
Replying to ChrisNelson:
My durations don't match the examples of good and bad behavior. 2690 is 5 days in the screen shots but 1 day on my chart.
... I've attached it in attachment:hours.csv.
My durations are more like yours now:
comment:14 Changed 12 years ago by
The attached log shows a very different WBS from the one in my local system. The log is consistent with the order of tasks in the r12330 Gantt attached to #9648. It seems that the chart is sorting correctly by WBS after computing the WBS incorrectly. Some ties may be broken in subtle ways but surely the milestone should be the last item in the WBS (12) whereas the attached log shows it as 4 (the fourth top-level item).
comment:15 Changed 12 years ago by
There are two differences near WBS handling between the before and after revisions:
@@ -34,21 +34,14 @@ # a ticket's parent is not in the viewed tickets, consider it # top-level wbs = [ 1 ] - for t in self.tickets: - if self.pm.parent(t) == None \ - or self.pm.parent(t) == 0 \ - or self.pm.parent(t) not in self.ticketsByID.keys(): - wbs = _setLevel(t['id'], wbs, 1) + for tid in self.pm.roots(self.ticketsByID): + wbs = _setLevel(tid, wbs, 1) Invoked like: tasks = '' self.tickets = self._query_tickets(options) - # Post process the query to add and compute fields so - # displaying the tickets is easy - self.pm.postQuery(options, self.tickets) - # Faster lookups for WBS and scheduling. self.ticketsByID = {} for t in self.tickets:
It may be that roots()
returns tickets in a different order than tickets
(we pass in a hash, not a list).
The second change is because the postQuery()
was rolled into _query_tickets()
. I think we can safely ignore that.
comment:16 Changed 12 years ago by
I see exactly the "before" graph when I apply this patch to r12330:
-
tracjsgantt/tracjsgantt.py
a b All other macro arguments are treated as TracQuery specific 365 365 # a ticket's parent is not in the viewed tickets, consider it 366 366 # top-level 367 367 wbs = [ 1 ] 368 for tid in self.pm.roots(self.ticketsByID): 369 wbs = _setLevel(tid, wbs, 1) 368 roots = self.pm.roots(self.ticketsByID) 369 for t in self.tickets: 370 if t['id'] in roots: 371 wbs = _setLevel(t['id'], wbs, 1) 370 372 371 373 372 374 def _task_display(self, t, options):
if it works for you, I'll put it in the repo.
comment:17 Changed 12 years ago by
let's see on Monday... thanks already now for your great support :-)
comment:18 Changed 12 years ago by
Yes! Your patch fixes the whole problem. My chart is displayed like in r12213 again. :-)
comment:19 Changed 12 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
(In [12426]) Assign WBS in correct order. Refs #10686, #9648.
The order of WBS assignments was broken by r12314.
TracPM.roots() returns an unordered list of ticket IDs so traversing that list meant we were no longer computing the WBS in the correct order. Now we use that list to determine what is a root ticket but traverse the ticket list in the scheduled (sorted) order.
comment:20 follow-up: 21 Changed 12 years ago by
I still slightly wonder why the problem didn't hit you
comment:21 Changed 12 years ago by
Replying to falkb:
I still slightly wonder why the problem didn't hit you
I believe our charts may have been misordered and it never rose to the level of needing to be fixed here. What I don't understand is why I didn't see your behavior with your tickets. My theory is that ultimately, ticket ID may bea tie breaker when sorting and my IDs for your tickets were assigned in a different order than your originals.
comment:22 Changed 12 years ago by
Yes, but this is the only reason I can imagine because we synchonized as much as possible. And thanks for standing my permanent nagging ;)
comment:23 Changed 12 years ago by
Description: | modified (diff) |
---|
falkb is still using
fields.*
to configure TracPM for getting data from Master Tickets and Subtickets plugins. I see different behavior withrelation.*
than withfield.*
. I don't know why it should be different but I'll fix the problem withfields.*
first then worry about the discrepancy.