Modify

Opened 7 years ago

Closed 7 years ago

#2223 closed defect (fixed)

casting & division by zero issues

Reported by: mape Owned by: mape
Priority: normal Component: TimeVisualizerPlugin
Severity: normal Keywords:
Cc: mape 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 mape 7 years ago.
impl.py.patch (3.9 KB) - added by mape 7 years ago.
patch2860-impl.py (3.8 KB) - added by yserrano 7 years ago.
patch against rev 2860
patch_comparison.diff (1.2 KB) - added by mape 7 years ago.
differences between first and second patch

Download all attachments as: .zip

Change History (14)

Changed 7 years ago by mape

Changed 7 years ago by mape

comment:1 Changed 7 years ago by mape

  • Status changed from new to assigned

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 7 years ago by mape

  • Cc mape th07@… added

comment:3 Changed 7 years ago by mape

  • Cc th07@… removed

comment:4 Changed 7 years ago by mape

#2248 is probably duplicate

Changed 7 years ago by yserrano

patch against rev 2860

comment:5 follow-up: Changed 7 years ago by yserrano

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 ; follow-up: Changed 7 years ago by 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?

comment:7 Changed 7 years ago by mape

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 7 years ago by mape

differences between first and second patch

comment:8 in reply to: ↑ 6 ; follow-up: Changed 7 years ago by mape

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 7 years ago by yserrano

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 7 years ago by mape

  • Resolution set to fixed
  • Status changed from assigned to closed

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.

Add Comment

Modify Ticket

Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
Author


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

 
Note: See TracTickets for help on using tickets.