Skip to content

Commit f63becd

Browse files
committed
feat(s2dm): implement handling for skipped empty branches during export
Signed-off-by: JD Alvarez <8550265+jdacoello@users.noreply.github.com>
1 parent 884a158 commit f63becd

File tree

6 files changed

+276
-4
lines changed

6 files changed

+276
-4
lines changed

src/vss_tools/exporters/s2dm/metadata_tracker.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ def init_vspec_comments() -> dict[str, Any]:
2424
- field_deprecated: field_path -> "deprecation reason" for @deprecated directive
2525
- instance_tags: tag_name -> {"element": "BRANCH", "fqn": "...", "instances": "..."}
2626
- instance_tag_types: type_name -> tag_name
27+
- skipped_empty_branches: list of branches skipped during export (no children)
2728
2829
Returns:
2930
Initialized metadata tracking dictionary
@@ -35,6 +36,7 @@ def init_vspec_comments() -> dict[str, Any]:
3536
"field_vss_types": {},
3637
"field_ranges": {},
3738
"field_deprecated": {},
39+
"skipped_empty_branches": [],
3840
}
3941

4042

src/vss_tools/exporters/s2dm/reference_generator.py

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,9 +311,40 @@ def generate_vspec_reference(
311311
except (PermissionError, OSError) as e:
312312
raise S2DMExporterException(f"Failed to write pluralized fields file {pluralized_output}: {e}") from e
313313

314+
# Write skipped empty branches if any were collected
315+
if mapping_metadata and mapping_metadata.get("skipped_empty_branches"):
316+
not_mapped_output = reference_dir / "not_mapped.yaml"
317+
try:
318+
with open(not_mapped_output, "w") as f:
319+
# Write header comment
320+
f.write("# Branches Skipped During S2DM Export\n")
321+
f.write("#\n")
322+
f.write("# This file lists VSS branches that were not mapped to GraphQL types because\n")
323+
f.write("# they have no children (no properties or sub-branches). Empty branches cannot\n")
324+
f.write("# be represented as GraphQL types since GraphQL requires types to have at least\n")
325+
f.write("# one field.\n")
326+
f.write("#\n")
327+
f.write("# To include these branches in the schema, add properties or sub-branches to them\n")
328+
f.write("# in the VSS specification.\n\n")
329+
330+
f.write("skipped_empty_branches:\n")
331+
for entry in mapping_metadata["skipped_empty_branches"]:
332+
f.write(f" - fqn: {entry['fqn']}\n")
333+
f.write(f" graphql_type_name: {entry['graphql_type_name']}\n")
334+
f.write(f" reason: {entry['reason']}\n")
335+
f.write("\n")
336+
337+
count = len(mapping_metadata["skipped_empty_branches"])
338+
log.info(f" - Not mapped (empty branches): {not_mapped_output.name} ({count} branch(es))")
339+
except (PermissionError, OSError) as e:
340+
raise S2DMExporterException(f"Failed to write not mapped file {not_mapped_output}: {e}") from e
341+
314342
# Generate README.md for provenance documentation
315343
has_plural_warnings = bool(mapping_metadata and mapping_metadata.get("plural_type_warnings"))
316-
generate_reference_readme(reference_dir, vspec_file, actual_units, actual_quantities, has_plural_warnings)
344+
has_skipped_branches = bool(mapping_metadata and mapping_metadata.get("skipped_empty_branches"))
345+
generate_reference_readme(
346+
reference_dir, vspec_file, actual_units, actual_quantities, has_plural_warnings, has_skipped_branches
347+
)
317348

318349
except S2DMExporterException:
319350
# Re-raise our custom exceptions
@@ -329,6 +360,7 @@ def generate_reference_readme(
329360
units_files: tuple[Path, ...] | None,
330361
quantities_files: tuple[Path, ...] | None,
331362
has_plural_warnings: bool = False,
363+
has_skipped_branches: bool = False,
332364
) -> None:
333365
"""
334366
Generate README.md documenting the provenance of reference files.
@@ -339,6 +371,7 @@ def generate_reference_readme(
339371
units_files: Units files used (explicit or implicit)
340372
quantities_files: Quantities files used (explicit or implicit)
341373
has_plural_warnings: Whether plural type warnings were generated
374+
has_skipped_branches: Whether empty branches were skipped during export
342375
343376
Raises:
344377
S2DMExporterException: If README generation fails
@@ -377,6 +410,10 @@ def generate_reference_readme(
377410
readme_content += """
378411
* **plural_type_warnings.txt** - VSS branches with plural type names (GraphQL prefers singular)."""
379412

413+
if has_skipped_branches:
414+
readme_content += """
415+
* **not_mapped.yaml** - VSS branches skipped during export (empty branches with no children)."""
416+
380417
readme_content += """
381418
382419
## Documentation

src/vss_tools/exporters/s2dm/schema_generator.py

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,12 @@
2222
import pandas as pd
2323
from graphql import GraphQLField, GraphQLObjectType, GraphQLSchema, GraphQLString
2424

25-
from vss_tools.exporters.s2dm.graphql_utils import GraphQLElementType
25+
from vss_tools import log
26+
from vss_tools.exporters.s2dm.graphql_utils import GraphQLElementType, convert_name_for_graphql_schema
2627
from vss_tools.tree import VSSNode
2728
from vss_tools.utils.pandas_utils import detect_and_resolve_short_name_collisions, get_metadata_df
2829

29-
from .constants import CUSTOM_DIRECTIVES, S2DMExporterException
30+
from .constants import CUSTOM_DIRECTIVES, S2DM_CONVERSIONS, S2DMExporterException
3031
from .graphql_directive_processor import GraphQLDirectiveProcessor
3132
from .graphql_scalars import get_vss_scalar_types
3233
from .metadata_tracker import init_vspec_comments
@@ -132,6 +133,33 @@ def generate_s2dm_schema(
132133
# Create object types for all branches
133134
for fqn in branches_df.index:
134135
if fqn not in types_registry:
136+
# Check if branch is empty (has no children)
137+
node = tree.get_node_with_fqn(fqn)
138+
if node is None:
139+
log.error(f"Branch '{fqn}' not found in tree but present in DataFrame. Skipping.")
140+
continue
141+
142+
if node.is_leaf:
143+
# Branch has no children - skip creating empty type
144+
# Use same logic as create_object_type for type name
145+
if short_name_mapping and fqn in short_name_mapping:
146+
type_name = short_name_mapping[fqn]
147+
else:
148+
type_name = convert_name_for_graphql_schema(fqn, GraphQLElementType.TYPE, S2DM_CONVERSIONS)
149+
log.warning(
150+
f"Branch '{fqn}' (GraphQL type: '{type_name}') was skipped because "
151+
f"the reference vspec does not define any sub-elements inside."
152+
)
153+
# Track skipped branch for reporting
154+
vspec_comments["skipped_empty_branches"].append(
155+
{
156+
"fqn": fqn,
157+
"graphql_type_name": type_name,
158+
"reason": "Empty branch (no children)",
159+
}
160+
)
161+
continue
162+
135163
types_registry[fqn] = create_object_type(
136164
fqn,
137165
branches_df,

src/vss_tools/exporters/s2dm/type_builders.py

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -558,9 +558,25 @@ def get_fields() -> dict[str, GraphQLField]:
558558

559559
# Branch fields
560560
for child_fqn, child_row in branches_df[branches_df["parent"] == fqn].iterrows():
561+
# Check if child branch is empty (has no children)
562+
has_leaf_children = not leaves_df[leaves_df["parent"] == child_fqn].empty
563+
has_branch_children = not branches_df[branches_df["parent"] == child_fqn].empty
564+
565+
if not has_leaf_children and not has_branch_children:
566+
# Skip creating field for empty branches
567+
log.debug(f"Skipping field for empty branch '{child_fqn}' in type '{type_name}'")
568+
continue
569+
561570
field_name = convert_name_for_graphql_schema(child_row["name"], GraphQLElementType.FIELD, S2DM_CONVERSIONS)
562571
child_type = types_registry.get(child_fqn) or create_object_type(
563-
child_fqn, branches_df, leaves_df, types_registry, unit_enums, vspec_comments, extended_attributes
572+
child_fqn,
573+
branches_df,
574+
leaves_df,
575+
types_registry,
576+
unit_enums,
577+
vspec_comments,
578+
extended_attributes,
579+
short_name_mapping,
564580
)
565581
types_registry[child_fqn] = child_type
566582

tests/test_s2dm_exporter.py

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -789,3 +789,165 @@ def test_extended_attributes_in_metadata(self):
789789
assert '@vspec(element: ATTRIBUTE, fqn: "Vehicle.Info.Model"' in schema_str
790790
assert '{key: "customMetadata", value: "test_value"}' in schema_str
791791
assert '{key: "anotherAttribute", value: "42"}' in schema_str
792+
793+
def test_empty_branches_are_skipped(self, caplog):
794+
"""Test that empty branches (branches with no children) are skipped during export."""
795+
# Load vspec with empty branches
796+
tree, _ = get_trees(
797+
vspec=Path("tests/vspec/test_s2dm/test_empty_branches.vspec"),
798+
include_dirs=(),
799+
aborts=(),
800+
strict=False,
801+
extended_attributes=(),
802+
quantities=(Path("tests/vspec/test_s2dm/test_quantities.yaml"),),
803+
units=(Path("tests/vspec/test_s2dm/test_units.yaml"),),
804+
overlays=(),
805+
expand=False,
806+
)
807+
808+
# Generate schema
809+
schema, unit_metadata, allowed_metadata, vspec_comments = generate_s2dm_schema(tree, use_short_names=True)
810+
811+
# Check that empty branches are NOT in the schema type_map
812+
assert "EmptyBranch" not in schema.type_map
813+
assert "AnotherEmpty" not in schema.type_map
814+
assert "EmptyNested" not in schema.type_map
815+
816+
# Check that non-empty branches ARE in the schema
817+
assert "Vehicle" in schema.type_map
818+
assert "HasContent" in schema.type_map
819+
820+
# Generate the actual GraphQL SDL output
821+
schema_str = print_schema_with_vspec_directives(schema, unit_metadata, allowed_metadata, vspec_comments)
822+
823+
# Verify empty types don't appear in the SDL output
824+
assert "type EmptyBranch" not in schema_str
825+
assert "type AnotherEmpty" not in schema_str
826+
assert "type EmptyNested" not in schema_str
827+
828+
# Verify non-empty types DO appear in the SDL output
829+
assert "type Vehicle" in schema_str
830+
assert "type HasContent" in schema_str
831+
832+
# Verify HasContent has the expected speed field
833+
assert "speed" in schema_str.lower()
834+
835+
# Check that warnings were logged for empty branches
836+
assert "Vehicle.EmptyBranch" in caplog.text
837+
assert "was skipped because the reference vspec does not define any sub-elements inside" in caplog.text
838+
assert "Vehicle.AnotherEmpty" in caplog.text
839+
assert "Vehicle.HasContent.EmptyNested" in caplog.text
840+
841+
def test_empty_branches_skipped_with_fqn_names(self, caplog, tmp_path):
842+
"""Test that empty branches are skipped when using FQN names (use_short_names=False)."""
843+
# Load vspec with empty branches
844+
tree, _ = get_trees(
845+
vspec=Path("tests/vspec/test_s2dm/test_empty_branches.vspec"),
846+
include_dirs=(),
847+
aborts=(),
848+
strict=False,
849+
extended_attributes=(),
850+
quantities=(Path("tests/vspec/test_s2dm/test_quantities.yaml"),),
851+
units=(Path("tests/vspec/test_s2dm/test_units.yaml"),),
852+
overlays=(),
853+
expand=False,
854+
)
855+
856+
# Generate schema with FQN names
857+
schema, unit_metadata, allowed_metadata, vspec_comments = generate_s2dm_schema(tree, use_short_names=False)
858+
859+
# Check that empty branches are NOT in the schema type_map (FQN format)
860+
assert "Vehicle_EmptyBranch" not in schema.type_map
861+
assert "Vehicle_AnotherEmpty" not in schema.type_map
862+
assert "Vehicle_HasContent_EmptyNested" not in schema.type_map
863+
864+
# Check that non-empty branches ARE in the schema
865+
assert "Vehicle" in schema.type_map
866+
assert "Vehicle_HasContent" in schema.type_map
867+
868+
# Generate the actual GraphQL SDL output
869+
schema_str = print_schema_with_vspec_directives(schema, unit_metadata, allowed_metadata, vspec_comments)
870+
871+
# Write to file for debugging
872+
output_file = tmp_path / "test_empty_branches_fqn.graphql"
873+
output_file.write_text(schema_str)
874+
875+
# Verify empty types don't appear in the SDL output (FQN format)
876+
assert "type Vehicle_EmptyBranch" not in schema_str
877+
assert "type Vehicle_AnotherEmpty" not in schema_str
878+
assert "type Vehicle_HasContent_EmptyNested" not in schema_str
879+
880+
# Verify non-empty types DO appear in the SDL output
881+
assert "type Vehicle " in schema_str or "type Vehicle {" in schema_str or "type Vehicle\n" in schema_str
882+
assert "type Vehicle_HasContent" in schema_str
883+
884+
# Check that warnings were logged for empty branches
885+
assert "Vehicle.EmptyBranch" in caplog.text
886+
assert "Vehicle.AnotherEmpty" in caplog.text
887+
assert "Vehicle.HasContent.EmptyNested" in caplog.text
888+
889+
def test_empty_branches_reported_in_not_mapped_yaml(self, tmp_path):
890+
"""Test that empty branches are reported in vspec_reference/not_mapped.yaml."""
891+
from vss_tools.exporters.s2dm import generate_vspec_reference
892+
893+
# Load vspec with empty branches
894+
tree, _ = get_trees(
895+
vspec=Path("tests/vspec/test_s2dm/test_empty_branches.vspec"),
896+
include_dirs=(),
897+
aborts=(),
898+
strict=False,
899+
extended_attributes=(),
900+
quantities=(Path("tests/vspec/test_s2dm/test_quantities.yaml"),),
901+
units=(Path("tests/vspec/test_s2dm/test_units.yaml"),),
902+
overlays=(),
903+
expand=False,
904+
)
905+
906+
# Generate schema (captures skipped branches in vspec_comments)
907+
schema, unit_metadata, allowed_metadata, vspec_comments = generate_s2dm_schema(tree, use_short_names=True)
908+
909+
# Verify that skipped branches were tracked
910+
assert "skipped_empty_branches" in vspec_comments
911+
skipped = vspec_comments["skipped_empty_branches"]
912+
assert len(skipped) == 3
913+
914+
# Verify the FQNs of skipped branches
915+
skipped_fqns = [entry["fqn"] for entry in skipped]
916+
assert "Vehicle.EmptyBranch" in skipped_fqns
917+
assert "Vehicle.AnotherEmpty" in skipped_fqns
918+
assert "Vehicle.HasContent.EmptyNested" in skipped_fqns
919+
920+
# Generate vspec_reference with the metadata
921+
generate_vspec_reference(
922+
tree=tree,
923+
data_type_tree=None,
924+
output_dir=tmp_path,
925+
extended_attributes=(),
926+
vspec_file=Path("tests/vspec/test_s2dm/test_empty_branches.vspec"),
927+
units_files=(Path("tests/vspec/test_s2dm/test_units.yaml"),),
928+
quantities_files=(Path("tests/vspec/test_s2dm/test_quantities.yaml"),),
929+
mapping_metadata=vspec_comments,
930+
)
931+
932+
# Verify not_mapped.yaml was created
933+
not_mapped_file = tmp_path / "vspec_reference" / "not_mapped.yaml"
934+
assert not_mapped_file.exists()
935+
936+
# Verify contents of not_mapped.yaml
937+
content = not_mapped_file.read_text()
938+
assert "skipped_empty_branches:" in content
939+
assert "Vehicle.EmptyBranch" in content
940+
assert "Vehicle.AnotherEmpty" in content
941+
assert "Vehicle.HasContent.EmptyNested" in content
942+
assert "Empty branch (no children)" in content
943+
944+
# Verify header comments are present
945+
assert "Branches Skipped During S2DM Export" in content
946+
assert "no children (no properties or sub-branches)" in content
947+
948+
# Verify README mentions the file
949+
readme_file = tmp_path / "vspec_reference" / "README.md"
950+
assert readme_file.exists()
951+
readme_content = readme_file.read_text()
952+
assert "not_mapped.yaml" in readme_content
953+
assert "empty branches" in readme_content.lower()
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Test vspec with empty branches to verify they are skipped during S2DM export
2+
3+
Vehicle:
4+
type: branch
5+
description: High-level vehicle data.
6+
7+
Vehicle.EmptyBranch:
8+
type: branch
9+
description: A branch that does not have any sub-elements.
10+
11+
Vehicle.HasContent:
12+
type: branch
13+
description: A branch that has some elements inside.
14+
15+
Vehicle.HasContent.Speed:
16+
type: sensor
17+
datatype: float
18+
unit: percent
19+
description: Vehicle speed.
20+
21+
Vehicle.AnotherEmpty:
22+
type: branch
23+
description: Another empty branch for testing.
24+
25+
Vehicle.HasContent.EmptyNested:
26+
type: branch
27+
description: An empty branch nested inside a non-empty branch.

0 commit comments

Comments
 (0)