rsennrich / ParZu

The Zurich Dependency Parser for German
https://pub.cl.uzh.ch/demo/parzu/
GNU General Public License v2.0
81 stars 19 forks source link

Timeout values cannot be configured #28

Closed johann-petrak closed 5 years ago

johann-petrak commented 5 years ago

The timeout values for reading from the swipl processes is hard-coded to 10 seconds in parzu_class.py. I found a large number of cases where this was not enough.

It would be very helpful if these values could get changed via the options map that gets passed to the Parser.__init__ method.

rsennrich commented 5 years ago

timeout is now configurable via an extra argument to init (separate from the options map, so that the options remain re-usable between batch and server processing).

johann-petrak commented 5 years ago

Sorry, I do not understand this: why is the new timeout parameter separate and not part of the options? As an option one could add it to the command line arguments, and more importantly, when creating a Server instance, one could pass it through the add_options parameter. But if it is separate, how would code that creates a Server instance achieve modification of the timeout in the Parser instance?

My assumption would have been that adding something like swipl_timeout/--swipl-timeout to the options and then using this in Parser.init would be the easiest way to achieve this?

rsennrich commented 5 years ago

The timeout is now exposed as a command-line parameter in parzu_server. Forcing the options to be read from the command line when creating a server instance wasn't great for bigger projects, so I now changed the code to also allow passing the options (and the timeout) to the Server initializer.

Long-term, it would be nice to rewrite/refactor the options code using argparse, and sharing most of them between parzu.py and parzu_class.py