Modify

Opened 9 years ago

Closed 9 years ago

#2223 closed defect (fixed)

casting & division by zero issues

Reported by: Markus Pelkonen Owned by: Markus Pelkonen
Priority: normal Component: TimeVisualizerPlugin
Severity: normal Keywords:
Cc: Markus Pelkonen Trac Release: 0.10

Description

From: Magnus
Sent: 2007-10-17
Subject: Timevisualizerplugin patch

I had some problems making the plugin run on postgres (python2.4 and trac0.10).

The first problem was that 'casting' with +0.0 fails; I have replaces this with some python code that should work with all databases.

The next problem is that code like:

cursor.execute(sql).fetchall()

fails. I have replaces this with

cursor.execute(sql)
cursor.fetchall()

I also got some Division by Zero issues that I have fixed with some max() code.

Included in this mail is the resulting impl.py as well as a patch against 2665.

Attachments (4)

impl.py (13.5 KB) - added by Markus Pelkonen 9 years ago.
impl.py.patch (3.9 KB) - added by Markus Pelkonen 9 years ago.
patch2860-impl.py (3.8 KB) - added by Yves Serrano 9 years ago.
patch against rev 2860
patch_comparison.diff (1.2 KB) - added by Markus Pelkonen 9 years ago.
differences between first and second patch

Download all attachments as: .zip

Change History (14)

Changed 9 years ago by Markus Pelkonen

Attachment: impl.py added

Changed 9 years ago by Markus Pelkonen

Attachment: impl.py.patch added

comment:1 Changed 9 years ago by Markus Pelkonen

Status: newassigned

With a quick reading, the patch seems feasible.

I'm surprised that chained method calls cursor.execute(...).fetchall() doesn't work for u. But well, some say that it is good coding convention to break statements like that, thus I'll migrate... in next release.

comment:2 Changed 9 years ago by Markus Pelkonen

Cc: Markus Pelkonen th07@… added; anonymous removed

comment:3 Changed 9 years ago by Markus Pelkonen

Cc: th07@… removed

comment:4 Changed 9 years ago by Markus Pelkonen

#2248 is probably duplicate

Changed 9 years ago by Yves Serrano

Attachment: patch2860-impl.py added

patch against rev 2860

comment:5 Changed 9 years ago by Yves Serrano

the patch failed on my installation, added a new patch against 2860 and replaced the command line path with sys.argv[1]

comment:6 in reply to:  5 ; Changed 9 years ago by Markus Pelkonen

Replying to yserrano:

the patch failed on my installation

What was the error in detail?

added a new patch against 2860 and replaced the command line path with sys.argv[1]

Was this command line handling the only change?

comment:7 Changed 9 years ago by Markus Pelkonen

I couldn't ever reproduce the issue, not even in Debian Etch running inside Vmware player. Well, I sill used sqlite.

I've made new 0.5 release candidate under trunk. Please try it and let me know if I merged first patch properly. At least my M$ installation didn't break.

Changed 9 years ago by Markus Pelkonen

Attachment: patch_comparison.diff added

differences between first and second patch

comment:8 in reply to:  6 ; Changed 9 years ago by Markus Pelkonen

Replying to mape:

Replying to yserrano:

the patch failed on my installation

What was the error in detail?

added a new patch against 2860 and replaced the command line path with sys.argv[1]

Was this command line handling the only change?

I applied both patches afterwards and then compared those local files, result is patch_comparison.diff. Changes affect only __main__. I don't see why the first patch then would have failed but the second patch would solve issued. Thus I ignore the latter patch.

Indeed, hard coded values are bad, thus I created new ticket: #2291 because those ideas don't fall under topic of this ticket.

comment:9 in reply to:  8 Changed 9 years ago by Yves Serrano

Replying to mape:

I applied both patches afterwards and then compared those local files, result is patch_comparison.diff. Changes affect only __main__. I don't see why the first patch then would have failed but the second patch would solve issued. Thus I ignore the latter patch.

Indeed, hard coded values are bad, thus I created new ticket: #2291 because those ideas don't fall under topic of this ticket.

Strange, when I tried to apply your original patch on mac os x or under linux I got this results:

yserrano:/tmp/timevisualizerplugin$ svn info
URL: http://trac-hacks.org/svn/timevisualizerplugin/tags/TimeVisualizer_0.4
yserrano:/tmp/timevisualizerplugin$ patch -p0 < ../impl.py.patch 
(Stripping trailing CRs from patch.)
patching file tractimevisualizerplugin/impl.py
Hunk #1 FAILED at 90.
Hunk #2 FAILED at 135.
Hunk #3 FAILED at 155.
Hunk #4 FAILED at 241.
Hunk #5 FAILED at 291.
5 out of 5 hunks FAILED -- saving rejects to file tractimevisualizerplugin/impl.py.rej
yserrano:/tmp/timevisualizerplugin$ patch -p0 < ../patch2860-impl.py 
patching file tractimevisualizerplugin/impl.py
yserrano:/tmp/timevisualizerplugin$

but now this fixes are in the trunk, so I switched to the trunk version which works fine.

comment:10 Changed 9 years ago by Markus Pelkonen

Resolution: fixed
Status: assignedclosed

I don't know nix tools very well, e.g. never used patch tool. That my patch is actually svn diff, may differ from unified diff. Maybe you should have tried 'svn merge ...' instead.

But trunk works for you how, so I consider this issue fixed.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Markus Pelkonen.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.