Expose submodules and add type annotations - #67
Conversation
Type annotations
More type annotations
|
Converted to a draft while I test the new commit. |
Converted tests to UnitTest
tomsilver
left a comment
tomsilver
left a comment
There was a problem hiding this comment.
Thanks so much for doing this. Looks broadly good but I want to take a closer look before merging and @ronuchit may too. To make it a little easier to review, would you mind splitting this into at least three PRs:
- exposing the submodules
- changing the tests to unittests
- adding type annotations
| @@ -0,0 +1,11 @@ | |||
| # import os | |||
There was a problem hiding this comment.
remove this file or uncomment?
There was a problem hiding this comment.
Oops, will remove 😅
There was a problem hiding this comment.
Thanks so much for doing this. Looks broadly good but I want to take a closer look before merging and @ronuchit may too. To make it a little easier to review, would you mind splitting this into at least three PRs:
- exposing the submodules
- changing the tests to unittests
- adding type annotations
Sure! I'll link those here once they exist and ping you.
Unittests: #68
Expose submodules: #69
Type annotations: #70
| class PDDLParser: | ||
| """PDDL parsing class. | ||
| """ | ||
| predicates: Dict[str, Predicate] |
There was a problem hiding this comment.
I misread predicates as being set in one of PDDLParser's methods, I think, and just set the type def at the top of the class def.
Looking back at the code, predicates is actually set separately in PDDLProblemParser and PDDLDomainParser.
We could
- set the type hint in the parent
PDDLParserclass for both of them - Set it separately in each
- For
PDDLDomainParser,predicatesis set in_parse_domain_predicates, rather than the init itself. We could either set the type hint in_parse_domain_predicates, or the top of the class, like I did here (but forPDDLDomainParser.
- For
There was a problem hiding this comment.
But we can handle that in the split out PR.
Sure! I'll link those here once they exist and ping you. |
2 participants
Description
Make the
core,structs, andspacesmodules available for import. This allows importing libraries to use the contained classes for type annotations.Add some (not all) type annotations.
Tests
Ran each test file in
pddlgym/tests. They all passed.