-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add basic interface inheritance support (fixes #8) #9
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
from .block import Block, BlockInfo, BlockField, BlockFieldInfo | ||
from .block import (Block, BlockField, BlockFieldInfo, BlockInfo, | ||
get_inheritance_tree) | ||
|
||
__all__ = ["Block", "BlockInfo", "BlockField", "BlockFieldInfo"] | ||
__all__ = ["Block", "BlockInfo", "BlockField", "BlockFieldInfo", "get_inheritance_tree"] |
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,11 +1,13 @@ | ||||||||||||||||||
import os | ||||||||||||||||||
import re | ||||||||||||||||||
from typing import List, Literal, NamedTuple, Optional, Union | ||||||||||||||||||
from typing import Dict, List, Literal, NamedTuple, Optional, Set, Union | ||||||||||||||||||
|
||||||||||||||||||
from ..base import BaseInfo | ||||||||||||||||||
from ..constants import RESOLVER_TYPES, VALUE_TYPES | ||||||||||||||||||
from ..constants.block_fields import all_block_fields | ||||||||||||||||||
from ..dependency import Dependency, DependencyGroup | ||||||||||||||||||
from ..dependency import (Dependency, DependencyGroup, | ||||||||||||||||||
get_interface_dependencies, | ||||||||||||||||||
remove_interface_dependencies) | ||||||||||||||||||
from ..utils import pascal_case | ||||||||||||||||||
|
||||||||||||||||||
|
||||||||||||||||||
|
@@ -18,6 +20,14 @@ class BlockFieldInfo(NamedTuple): | |||||||||||||||||
value_type: str | ||||||||||||||||||
|
||||||||||||||||||
|
||||||||||||||||||
# a dictionary where for each node, we hold its children | ||||||||||||||||||
inheritanceTree: Dict[str, Set[str]] = {"root": set()} | ||||||||||||||||||
|
||||||||||||||||||
|
||||||||||||||||||
def get_inheritance_tree(): | ||||||||||||||||||
return inheritanceTree | ||||||||||||||||||
|
||||||||||||||||||
|
||||||||||||||||||
class Block(BaseInfo): | ||||||||||||||||||
def __init__(self, info, dependency_group: DependencyGroup) -> None: | ||||||||||||||||||
super().__init__(info) | ||||||||||||||||||
|
@@ -35,6 +45,7 @@ def display_name(self): | |||||||||||||||||
|
||||||||||||||||||
@property | ||||||||||||||||||
def heading_file_line(self): | ||||||||||||||||||
global inheritanceTree | ||||||||||||||||||
display_name = self.display_name | ||||||||||||||||||
|
||||||||||||||||||
if self.type == "enum": | ||||||||||||||||||
|
@@ -59,15 +70,28 @@ def heading_file_line(self): | |||||||||||||||||
) | ||||||||||||||||||
|
||||||||||||||||||
if not self.implements: | ||||||||||||||||||
# check if we have an interface implementing another interface | ||||||||||||||||||
deps = get_interface_dependencies() | ||||||||||||||||||
if display_name in deps: | ||||||||||||||||||
siblings = inheritanceTree.get(deps[display_name], set()) | ||||||||||||||||||
siblings.add(display_name) | ||||||||||||||||||
inheritanceTree[deps[display_name]] = siblings | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
https://docs.python.org/3.11/library/stdtypes.html#dict.setdefault or if you do
Suggested change
|
||||||||||||||||||
return f"@dataclass(kw_only=True)\nclass {display_name}({deps[display_name]}):" | ||||||||||||||||||
|
||||||||||||||||||
inheritanceTree["root"].add(display_name) | ||||||||||||||||||
return ( | ||||||||||||||||||
f"@dataclass(kw_only=True)\nclass {display_name}(DataClassJSONMixin):" | ||||||||||||||||||
) | ||||||||||||||||||
|
||||||||||||||||||
for el in self.info.implements.split("&"): # type: ignore | ||||||||||||||||||
self.parent_classes.add(el.strip()) | ||||||||||||||||||
|
||||||||||||||||||
parent = ", ".join(list(self.parent_classes)) | ||||||||||||||||||
return f"@dataclass(kw_only=True)\nclass {display_name}({parent}):" | ||||||||||||||||||
parents = remove_interface_dependencies( | ||||||||||||||||||
[x.strip() for x in self.info.implements.split("&")] # type: ignore | ||||||||||||||||||
) | ||||||||||||||||||
for p in parents: | ||||||||||||||||||
siblings = inheritanceTree.get(p, set()) | ||||||||||||||||||
siblings.add(display_name) | ||||||||||||||||||
inheritanceTree[p] = siblings | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. similar pattern as above with either |
||||||||||||||||||
parent_str = ", ".join(parents) | ||||||||||||||||||
return f"@dataclass(kw_only=True)\nclass {display_name}({parent_str}):" | ||||||||||||||||||
|
||||||||||||||||||
@property | ||||||||||||||||||
def category(self): | ||||||||||||||||||
|
@@ -99,8 +123,8 @@ def file_representation(self): | |||||||||||||||||
parent_fields = set() | ||||||||||||||||||
for p in self.parent_classes: | ||||||||||||||||||
parent_fields.update(all_block_fields.get(p, set())) | ||||||||||||||||||
|
||||||||||||||||||
for f in self.fields: | ||||||||||||||||||
# don't re-include parent fields | ||||||||||||||||||
if str(f).split(":")[1].strip() in parent_fields: | ||||||||||||||||||
continue | ||||||||||||||||||
|
||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,12 @@ | ||
from .dependency import Dependency, DependencyGroup | ||
from .dependency import (Dependency, DependencyGroup, | ||
get_interface_dependencies, | ||
remove_interface_dependencies, | ||
update_interface_dependencies) | ||
|
||
__all__ = ["Dependency", "DependencyGroup"] | ||
__all__ = [ | ||
"Dependency", | ||
"DependencyGroup", | ||
"get_interface_dependencies", | ||
"update_interface_dependencies", | ||
"remove_interface_dependencies", | ||
] |
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
@@ -1,12 +1,83 @@ | ||||||||
from itertools import groupby | ||||||||
from typing import List, NamedTuple, Set | ||||||||
from typing import Dict, List, NamedTuple, Set | ||||||||
|
||||||||
|
||||||||
class Dependency(NamedTuple): | ||||||||
imported_from: str | ||||||||
dependency: str | ||||||||
|
||||||||
|
||||||||
INTERMEDIATE_INTERFACES: Dict[str, str] = {} | ||||||||
|
||||||||
|
||||||||
def get_interface_dependencies(): | ||||||||
return INTERMEDIATE_INTERFACES | ||||||||
|
||||||||
|
||||||||
def update_interface_dependencies(config_file_content): | ||||||||
global INTERMEDIATE_INTERFACES | ||||||||
if type(config_file_content) is dict: | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
doing type check with >>> class Foo(dict): pass
>>> type(Foo()) is dict
False
>>> isinstance(Foo(), dict)
True There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know but I am explicitly followed the style of the rest of the repo here in the thought that we will perhaps refactor everything together There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if you do the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it makes sense to make the change - leaving extra bad code in was not great and leaves more work for potential future refactorings |
||||||||
data = config_file_content.get("interfaceInheritance") | ||||||||
if type(data) is dict: | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as above |
||||||||
INTERMEDIATE_INTERFACES = data | ||||||||
|
||||||||
|
||||||||
def remove_interface_dependencies( | ||||||||
interfaces: List[str], | ||||||||
intermediate_interfaces: Dict[str, str] = {}, | ||||||||
) -> List[str]: | ||||||||
"""Filter all dependencies from intermediate interfaces | ||||||||
|
||||||||
Assumes that all keys in intermediate_interfaces are leaf nodes and | ||||||||
returns all other parent interfaces from a given list | ||||||||
|
||||||||
Intemediate interfaces in GraphQL implement other interfaces. This is part | ||||||||
of the spec (see https://github.com/graphql/graphql-spec/pull/373) | ||||||||
but not implemented in all clients yet (e.g., neo4j) | ||||||||
Thus we are parsing intermediate interfaces in scalars.yml so as to emit | ||||||||
proper python code without relying on the graphql schema being correct | ||||||||
|
||||||||
|
||||||||
>>> remove_interface_dependencies(['t1', 't2', 'i1', 'i2', 't3'], {'i1':'i2'}) | ||||||||
['t1', 't2', 'i1', 't3'] | ||||||||
|
||||||||
>>> remove_interface_dependencies(['t1', 't2', 't3'], {}) | ||||||||
['t1', 't2', 't3'] | ||||||||
|
||||||||
>>> remove_interface_dependencies(['t1', 't2', 'i1', 'i2', 'i3', 't3'], \ | ||||||||
{'i1':'i2', 'i2': 'i3'}) | ||||||||
['t1', 't2', 'i1', 't3'] | ||||||||
|
||||||||
>>> remove_interface_dependencies(['t1', 't2', 'i1', 'i2', 'i3', \ | ||||||||
'ni1', 'ni2', 't3'], \ | ||||||||
{'i1':'i2', 'i2': 'i3', 'ni1':'ni2'}) | ||||||||
['t1', 't2', 'i1', 'ni1', 't3'] | ||||||||
""" | ||||||||
deps: Set[str] = set() | ||||||||
if not intermediate_interfaces: | ||||||||
intermediate_interfaces = INTERMEDIATE_INTERFACES | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
usually simplifies things a bit, especially if you count the mccabe function complexity its nice to reduce any direct if statements to more simple alternatives |
||||||||
for i in interfaces: | ||||||||
if i not in intermediate_interfaces: | ||||||||
# this is not an intermediate interface, thus we are keeping it | ||||||||
# as a dependency | ||||||||
continue | ||||||||
|
||||||||
# add the parent dependency to the list of tracked dependencies: | ||||||||
deps.add(intermediate_interfaces[i]) | ||||||||
|
||||||||
# transitively fetch all dependencies | ||||||||
seen: Set[str] = set() | ||||||||
while deps: | ||||||||
d = deps.pop() | ||||||||
if d in seen: | ||||||||
continue | ||||||||
if d in intermediate_interfaces: | ||||||||
deps.add(intermediate_interfaces[d]) | ||||||||
seen.add(d) | ||||||||
|
||||||||
return list(filter(lambda x: x not in seen, interfaces)) | ||||||||
|
||||||||
|
||||||||
class DependencyGroup: | ||||||||
def __init__( | ||||||||
self, deps: Set[Dependency] = set(), direct_deps: Set[str] = set() | ||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://docs.python.org/3.11/library/collections.html#collections.defaultdict
then whatever you access on the dict is always present automatically