Update GBML Config Wrapper to house {node|edge}_type_to_*_maps.#643
Update GBML Config Wrapper to house {node|edge}_type_to_*_maps.#643svij-sc wants to merge 2 commits into
Conversation
svij-sc
requested review from
kmontemayor2-sc,
mkolodner-sc,
nshah-sc,
xgao4-sc,
yliu2-sc and
zfan3-sc
as code owners
| "mdformat==0.7.22", | ||
| "mdformat_tables==1.0.0", | ||
| "ty~=0.0.29", | ||
| "ty==0.0.31", |
There was a problem hiding this comment.
Newer ty is more restrictive; so actually locking it.
| "_node_type_to_feature_dim_map", | ||
| { | ||
| graph_metadata.condensed_node_type_to_node_type_map[c]: d | ||
| for c, d in preprocessed_metadata.condensed_node_type_to_feature_dim_map.items() |
There was a problem hiding this comment.
nit: pref to use better var names than c and d. Ditto elsewhere.
| return self._preprocessed_metadata_pb_wrapper | ||
|
|
||
| @property | ||
| def node_type_to_feature_dim_map(self) -> dict[NodeType, int]: |
There was a problem hiding this comment.
For better cohesion, I think these fields belong on PreprocessedMetadataPbWrapper rather than GbmlConfigPbWrapper. Many GiGL functions — like this one — accept PreprocessedMetadataPbWrapper and GraphMetadataPbWrapper directly (not GbmlConfigPbWrapper), so they can't benefit from this change as currently placed. Putting the typed-keyed maps on PreprocessedMetadataPbWrapper (via a method that accepts a GraphMetadataPbWrapper) would make them reachable anywhere the preprocessed metadata is available, and would let callers like the linked function simplify their internal condensed-typed translation.
2 participants
Prior to this change one would need to manually convert
condensed_{node|edge}_type_*_mapsmanually to{node|edge}_type_to_*_mapsusing information available ingraph_metadata. This causes a lot of duplicity in code when we can just create convenience functions in the gbml wrapper.