On Fri, May 27, 2011 at 11:21 AM, Vossen, Bodo
<bodo.vos...@mpi-bn.mpg.de> wrote:
>
> Hi all,
>
> I've implemented functionality for adding tools via the admin menu.

That sounds very useful in principle, although perhaps
the less flexible alternative of reloading tool_conf.xml
would achieve the same aim?

> ...
> Then it is checked if script and XML are "compatible" by
> checking if they have the same number of arguments.

This seems very fragile, and having looked at the code for
detecting the number of arguments in a Python script it
looks like it will get it wrong in many situations. Even
for something quite simple like this:

#sys.argv[0] is the script file itself
assert len(sys,argv)==4, "Expect 3 arguments"
arg1, arg2, arg3 = sys.argv[1:]

Also, what about conditional code like this?

assert len(sys.argv)==4, "Expect 3 arguments"
if sys.argv[1] == "db":
    database = sys.argv[2]
    filename = None
else:
    database = None
    filename = sys.argv[2]
out_filename = sys.argv[3]

If I have read your get_parameter_number code right,
it would claim there are four arguments since there
are four lines of the basic form variable = sys.argv[i]

Also, it cannot possibly work when there are a varying
number of arguments, for example a repeat argument.
If you want a test case, my Venn Diagram tool on the
Tool Shed should be relevant (its a python script).

> If you have any comments or question please do not
> hesitate to contact me, I'm open for every kind of critics.

I would remove the attempt to check the number of arguments.
It is practically impossible to get this right in all cases,
so I don't think it is worth doing. Having valid script/xml
combinations rejected would be quite annoying.

Peter
___________________________________________________________
Please keep all replies on the list by using "reply all"
in your mail client.  To manage your subscriptions to this
and other Galaxy lists, please use the interface at:

  http://lists.bx.psu.edu/

Reply via email to