Issue1068

Title Allow importing pddl_parser without parsing arguments from command line
Priority feature Status resolved
Superseder Nosy List gabi, jendrik, malte
Assigned To Keywords
Optional summary

Created on 2022-10-18.10:40:18 by gabi, last changed by gabi.

Messages
msg10854 (view) Author: gabi Date: 2022-10-19.20:56:12
If you can invest the time to give the feedback, I can invest the time to 
incorporate it. :-D

The change is now merged to main. Thanks Jendrik and Malte!
msg10853 (view) Author: malte Date: 2022-10-19.16:14:45
If you decide to act on Jendrik's comments (and only then), I'd suggest also one more small clean-up: in all places but one, the order is "domain, then task", but in one it's "task, then domain". I'd make it "domain, then task" consistently.
msg10852 (view) Author: jendrik Date: 2022-10-19.16:12:24
I left a few nitpick comments, feel free to ignore. Looks good to merge.
msg10851 (view) Author: gabi Date: 2022-10-19.14:55:33
I went for the first option, moving "import options" into pddl_file.open. I also 
followed Malte's suggestion to only import if not all file names have been 
specified. The small pull request can be found here: 
https://github.com/aibasel/downward/pull/124
msg10850 (view) Author: malte Date: 2022-10-18.10:57:06
I don't like the second option much. "open" is the only function in pddl_parser.__init__, and it is supposed to be the only public interface of the package within our own code. It would be nice to keep it this way. Also, with that solution you could still not use "parse_pddl_file" because importing pddl_file.py would still trigger the option parsing. (Even if you don't need parse_pddl_file, I think a solution that allows using the other functions but not this one is not ideal.)

I think moving "import options" into pddl_file.open is a reasonable solution. I suggest adding a comment to explain why it is done this way. If you want, you can go one step further and only "import options" if the options are used, i.e., if either of the filename arguments are None/falsy. Then it's also possible to use "open" without the option parser by passing in two filenames.
msg10849 (view) Author: gabi Date: 2022-10-18.10:40:18
I would like to use some functionality of the Fast Downward translator directly from 
external code (related to AIPlan4EU).

My problem is that I cannot import anything from the pddl_parser package because the 
exposure of the open function from pddl_file.py imports options.py, which in turn 
initiates parsing arguments from the command line.

One possible solution would be to move the "import options" in pddl_file.py into the 
open function itself (which I don't plan to use, working with strings instead of 
intermediate files).

A alternative comparatively small change to resolve this would be to remove open from 
pddl_parser's __init__.py and replace the handful of calls of "pddl_parser.open()" with 
"pddl_parser.pddl_file.open()". This would not look as nicely anymore because it also 
requires to import pddl_parser.pddl_file instead of pddl_parser.

Comments? Alternative suggestions?
History
Date User Action Args
2022-10-19 20:56:12gabisetstatus: reviewing -> resolved
messages: + msg10854
2022-10-19 16:14:45maltesetmessages: + msg10853
2022-10-19 16:12:24jendriksetmessages: + msg10852
2022-10-19 14:55:33gabisetstatus: chatting -> reviewing
messages: + msg10851
2022-10-18 10:57:06maltesetstatus: unread -> chatting
nosy: + malte, jendrik
messages: + msg10850
2022-10-18 10:40:18gabicreate