Modify

Opened 13 years ago

Closed 13 years ago

#8484 closed defect (fixed)

Syntax error

Reported by: anonymous Owned by: Chris Nelson
Priority: normal Component: TracJsGanttPlugin
Severity: major Keywords:
Cc: Danny Sauer Trac Release: 0.12

Description

Plugin doesn't seem to work on a regular RHEL 5.4 system, using Python 2.4.3 and running Trac 0.12. It generates this error:

Feb  8 15:51:13 *hostname* Trac[loader] ERROR: Skipping "tracjsgantt = tracjsgantt":  Traceback (most recent call last):   File "/usr/lib/python2.4/site-packages/Trac-0.12-py2.4.egg/trac/loader.py", line 70, in _load_eggs     entry.load(require=True)   File "/usr/lib/python2.4/site-packages/pkg_resources.py", line 1830, in load     entry = __import__(self.module_name, globals(),globals(), ['__name__'])   File "build/bdist.linux-x86_64/egg/tracjsgantt/__init__.py", line 1, in ?     #   File "/usr/lib/python2.4/site-packages/Trac_jsGantt-0.3-py2.4.egg/tracjsgantt/tracjsgantt.py", line 133      return (options[name] if options.get(name) else self.options[name])                             ^ SyntaxError: invalid syntax (tracjsgantt.py, line 133))

I'm using the current SVN code:

$ svn info tracjsgantt/tracjsgantt.py | grep -i rev
Revision: 9844
Last Changed Rev: 9782

Attachments (2)

ternery.patch (1.8 KB) - added by Danny Sauer 13 years ago.
Udiff patch for tracjsgantt.py against SVN version 9849
oldpython.patch (5.2 KB) - added by Danny Sauer 13 years ago.
Patch to make plugin work with Python 2.4

Download all attachments as: .zip

Change History (12)

comment:1 Changed 13 years ago by Danny Sauer

Cc: Danny Sauer added; danny@… removed

comment:2 Changed 13 years ago by Chris Nelson

Resolution: wontfix
Status: newclosed

I'm sorry to say I'm running Trac 0.11 and Python 2.5.2 and no good way to try to reproduce your problem.

While I have done some work since the version you reference, the line:

return (options[name] if options.get(name) else self.options[name])

is still there and works fine for me.

http://en.wikipedia.org/wiki/Ternary_operation#Python notes that the ternary if was added to Python in 2.5. I believe you need a newer Python to use this plugin.

You could try replacing

        def g(name):
            return (options[name] if options.get(name) else self.options[name])

with

        def g(name):
            if options.get(name):
                return options[name]
            else:
                return self.options[name]

(Untested.) but I believe I use the ternary if elsewhere, too.

comment:3 Changed 13 years ago by osimons

I just spotted this by accident, but can't help but comment... Why don't you just use the common Python ideom of:

return options.get(name) or self.options[name])

This is the same case that presumes that options.get(name) is defined AND that it evaluates to True by having non-empty content. If you want it to return possibly empty values, the correct way would be to always return it when defined and only resort to some other default value if non-existing:

return options.get(name, self.options[name])
# alternatively
return name in options and options[name] or self.options[name]

Seeing Trac 0.11 even supports Python 2.3, using 2.5 syntax is not recommended. The patterns above is something you'll find in all parts of Trac when needed. The pattern works by the fact that Python will always return the value of the last and so that if several facts evaluates to True the last one is output for further use - and if any evaluates to False the remaining checks are skipped with no danger of things like lookup errors, and instead moving on to evaluate the or argument:

>>> a = 1
>>> a and 'yes' or 'no'
'yes'
>>> a = 0
>>> a and 'yes' or 'no'
'no'

comment:4 in reply to:  3 Changed 13 years ago by Chris Nelson

Replying to osimons:

I just spotted this by accident, but can't help but comment... Why don't you just use the common Python ideom of: ...

Because I have 25+ years programming experience but something like 25 days of it is with Python. Thanks for the tip!

comment:5 Changed 13 years ago by anonymous

I was going to make the same "and-or" suggestion, but since it's already been made...

I would be happy to test any modified code if you don't have an available test environment.

Changed 13 years ago by Danny Sauer

Attachment: ternery.patch added

Udiff patch for tracjsgantt.py against SVN version 9849

comment:6 Changed 13 years ago by Danny Sauer

Ok, that patch didn't do it. I'm still working. :)

Changed 13 years ago by Danny Sauer

Attachment: oldpython.patch added

Patch to make plugin work with Python 2.4

comment:7 Changed 13 years ago by Danny Sauer

Resolution: wontfix
Status: closedreopened

Ok, I created a working patch: attachment:oldpython.patch.

I incorporated the earlier partial patch, added some code to handle date time parsing in a way that works with 2.4, and had to modify the _work_time logic to skip milestones (I was getting an invalid key error since milestones don't have estimated hours).

I also tweaked it a little to allow tickets with just predecessors, not successors (in the event someone sets up a custom files instead of using the module which keeps the paired fields in sync), and replaced three tabs with whitespace for indentation consistency. ;)

I'm using this code now, and it works well. I haven't tested start and end dates with it yet, though, and I don't use parent tickets. Hopefully that stuff will just work. /crosses fingers

comment:8 Changed 13 years ago by Chris Nelson

(In [10237]) Removed ternary ifs. Refs #8484

comment:9 Changed 13 years ago by Chris Nelson

(In [10253]) Changed date handling to be compatible with Python 2.4. Refs #8484

comment:10 Changed 13 years ago by Chris Nelson

Resolution: fixed
Status: reopenedclosed

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Chris Nelson.
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.