Add support for including IDD \group in the schema #8426
Conversation
There was a problem hiding this comment.
@mbadams5 let me know if you spot any issues. This has been a tiny lift that I think would add some value to the schema.
| @@ -99,6 +99,7 @@ class Data: | |||
| def parse_idd(data): | |||
| root = {'$schema': "http://json-schema.org/draft-04/schema#", 'properties': {}} | |||
| data.file_size = len(data.file) | |||
| current_group_name = "**ungrouped**" | |||
There was a problem hiding this comment.
The IDD currently starts with a group at the top, but in case that ever gets botched, this would be the group name assigned to the objects ahead of the group declaration. More error handling could be added, but in this particular case I think it would be unnecessary to overthink this.
| @@ -110,15 +111,15 @@ def parse_idd(data): | |||
| elif token == TOKEN_EXCLAMATION: | |||
| eat_comment(data) | |||
| elif token == TOKEN_GROUP: | |||
| next_token(data) | |||
| eat_comment(data) | |||
| current_group_name = parse_group(data).replace('\\group ', '') | |||
There was a problem hiding this comment.
@mbadams5 I know this isn't perfect, and that I should just leverage one of your functions to "eat" the \group part, but this does work properly, so should just take a small tweak to refine it.
| else: | ||
| obj_name = parse_string(data) | ||
| if obj_name is None or obj_name == "": | ||
| return root | ||
| obj_data = parse_obj(data) | ||
| root['properties'][obj_name] = {} | ||
| root['properties'][obj_name]['patternProperties'] = {} | ||
| root['properties'][obj_name]['group'] = current_group_name |
There was a problem hiding this comment.
I used the field name "group", but would be open to discussion if we want something better.
| @@ -725,6 +726,18 @@ def eat_whitespace(data): | |||
| break | |||
|
|
|||
|
|
|||
| def parse_group(data): | |||
There was a problem hiding this comment.
I rolled a slightly modified parser worker function that eats until the end of the line. This could certainly eat the \group part if that's the preferred approach. If there is another worker function that could also do this, I'm all for it.
| @@ -111,14 +112,17 @@ def parse_idd(data): | |||
| eat_comment(data) | |||
| elif token == TOKEN_GROUP: | |||
| next_token(data) | |||
| eat_comment(data) | |||
| current_group_name = parse_line(data) | |||
There was a problem hiding this comment.
I figured I could leverage the existing ones, but I wasn't immediately sure. This is great.
| else: | ||
| obj_name = parse_string(data) | ||
| if obj_name is None or obj_name == "": | ||
| return root | ||
| obj_data = parse_obj(data) | ||
| root['properties'][obj_name] = {} | ||
| root['properties'][obj_name]['patternProperties'] = {} | ||
| if current_group_name is not None: |
There was a problem hiding this comment.
Initializing to None and checking that here is great...but if you reset it to None in here, only the first object in the group will get this tag, right?
There was a problem hiding this comment.
You are correct. I forgot there was only one group declaration for multiple EP objects.
| @@ -19,5 +20,5 @@ | |||
| modify_schema.change_89_release_issues(data.schema) | |||
| modify_schema.add_explicit_extensible_bounds(data.schema) | |||
|
|
|||
| with open(source_dir_path + '/idd/Energy+.schema.epJSON.in', 'w') as f2: | |||
| with open(path.join(source_dir_path, 'idd', 'Energy+.schema.epJSON.in'), 'w') as f2: | |||
There was a problem hiding this comment.
Always a nice improvement.
|
Thanks for fixing this up @mbadams5 ! Anyone have any issues why the group key shouldn't be included in the schema? If not I'll merge this in later on. |
|
Thanks again @mbadams5 for final polishing on this. |
The IDD includes a
\groupsection header that allows interfaces to automatically get object categorization when presenting the hundreds of IDD obejcts. With the new schema, the group is lost, and the interface must maintain their own list or use another algorithm to determine categories. With this PR, as the IDD is being parsed, the "current" group is tracked, and included in each schema object. This adds one field to each IDD object in the schema, but I think that interfaces will appreciate being able to read this category rather than having to maintain a separate list.