search

delph-in / pydelphin

Python libraries for DELPH-IN
https://pydelphin.readthedocs.io/
MIT License
79 stars 27 forks source link

ACEGenerator and ACETransferer out of sync with delphin.commands.process() #278

Closed goodmami closed 4 years ago

goodmami commented 4 years ago

delphin.commands.process() calls the processor with the stderr argument for redirecting ACE's stderr. The ACEGenerator and ACETransferer did not accept this argument because, for a while, they relied on stderr to interpret the results. Now they give a TypeError when called from process() because of the unexpected argument.

There are still some arguments they do not uniformly take, such as tsdbinfo (not allowed on ACETransferer) and full_forest (only allowed on ACEParser)

arademaker commented 4 years ago

how to deal with these arguments not uniformly take? Is it an issue for ACE, right?

goodmami commented 4 years ago

@arademaker no it's an issue with PyDelphin. I have not been testing transfer and generation from the delphin.commands.process() function or the delphin process command and there was a regression in the way they call the respective classes in delphin.ace. Nothing was broken if you did generation or transfer using delphin.ace directly, only when you tried to do those tasks from process().

The stderr parameter allows you to redirect ACE's stderr stream to some other stream or file. The benefit over doing 2>ace.err at the command line is that you can avoid getting PyDelphin stderr messages as well.

The fix is easy; I just came across the bug when testing the last release (unfortunately, after it was released).