Loading activity from json #28
Conversation
Signed-off-by: Aitor Miguel Blanco <[email protected]>
Signed-off-by: Aitor Miguel Blanco <[email protected]>
Signed-off-by: Aitor Miguel Blanco <[email protected]>
pycozmo/activity.py
Outdated
| def from_json(cls, data: Dict): | ||
| return cls( | ||
| choice_type=data['type'], | ||
| behaviors=data['behaviors'] if 'behaviors' in data else [] |
There was a problem hiding this comment.
Consider the following in such cases:
behaviors=data.get('behaviors', [])
There was a problem hiding this comment.
This is so much nicer, thanks a lot!
pycozmo/activity.py
Outdated
| # TODO: add score based choice method | ||
| pass | ||
| else: | ||
| print(self.choice_type) |
pycozmo/activity.py
Outdated
| ignore_if_locked=data['ignoreIfLocked'], | ||
| probability_to_require_objective=data['probabilityToRequireObjective'], | ||
| random_completions_needed_min=data['randomCompletionsNeededMin'] | ||
| if 'randomCompletionsNeededMin' in data else None, |
There was a problem hiding this comment.
If the default is 0, why pass None here?
There was a problem hiding this comment.
This is the same as `random_completions_needed_min=data.get('randomCompletionsNeededMin')
There was a problem hiding this comment.
It's None due to copy-paste or autocomplete most likely. I'll correct it.
| activity_id=info['activityID'], | ||
| activity_type=info['activityType'], | ||
| strategy=info['activityStrategy']['type'] | ||
| ) |
There was a problem hiding this comment.
There is one thing I wanted to ask. In this class, you added some slots. Should I include slots for the non-optional variables in the different activities?
There was a problem hiding this comment.
__slots__ is a way to restrict what fields can be used in a class. Feel free to remove them for now for activities.
pycozmo/activity.py
Outdated
| def get_activity_files(resource_dir: str) -> List[str]: | ||
| activity_files = [resource_dir + '/cozmo_resources/config/engine/behaviorSystem/activities_config.json'] | ||
| activity_folder = resource_dir + '/cozmo_resources/config/engine/behaviorSystem/activities/' | ||
| for root, dirs, files in os.walk(activity_folder): |
There was a problem hiding this comment.
As you are not using dirs, use _ instead.
| if '//' in line: | ||
| wordlist = line.split(' ') | ||
| # get all words before '//' | ||
| line = ' '.join(wordlist[:wordlist.index('//')]) |
There was a problem hiding this comment.
Uhh, they are using malformed JSON :/
There was a problem hiding this comment.
Yes, quite annoying haha I hope they are only using this format for the comments.
Signed-off-by: Aitor Miguel Blanco <[email protected]>
|
Looks good. Merge as is? |
|
Can be merged, yes, I'll move to another set of files. |
This is a first try on loading activity maps from JSON files.
I wanted to create the PR to ask for some feedback about the implementation, please let me know if I can do things in a nicer way.
Currently, no logic on the activities is done, only the mapping from JSON into different classes.