Conversation
…t will group by the label (if it exists). Yo can also specify the property to groupby using the switch --groupby or -g followed by the property name
…t will group by the label (if it exists). Yo can also specify the property to groupby using the switch --groupby or -g followed by the property name
…und an incorrect case of grouping which was fixed and a test added
… group field was not in the expected results
austinkline
left a comment
austinkline
left a comment
There was a problem hiding this comment.
I think we can rework some of this logic so that the defaults all line up together, let's see if that's a viable strategy then circle back
| parser.add_argument('query_mode', nargs='?', default='query', | ||
| help='query mode (default=query) [query|explain|profile]') | ||
| parser.add_argument('-p', '--path-pattern', default='', help='path pattern') | ||
| parser.add_argument('-g', '--groupby', default='', help='group by property') |
There was a problem hiding this comment.
parser.add_argument('-g', '--group-by', default=T_LABEL, help='group by property')
|
|
||
| def __init__(self, graph: MultiDiGraph = None, callbacks=None, label_max_length=DEFAULT_LABEL_MAX_LENGTH): | ||
| def __init__(self, graph: MultiDiGraph = None, callbacks=None, label_max_length=DEFAULT_LABEL_MAX_LENGTH, | ||
| group_by_property=None): |
There was a problem hiding this comment.
We should be able to just set T_LABEL as the default here, then rework some of the calling logic.
def __init__(self, graph: MultiDiGraph = None, callbacks=None, label_max_length=DEFAULT_LABEL_MAX_LENGTH,
group_by_property=T_LABEL):
| if graph is None: | ||
| graph = MultiDiGraph() | ||
| self.label_max_length = label_max_length | ||
| if group_by_property: |
There was a problem hiding this comment.
This would all go away to just be
self.graph_by_property = group_by_property
| from networkx import MultiDiGraph | ||
|
|
||
| logging.basicConfig() | ||
| logger = logging.getLogger("graph_magic") |
There was a problem hiding this comment.
logger = logging.getLogger("GremlinNetwork")
or
logger = logging.getLogger(__file__)
| # If the group_by_property is specified and it exists then assign that property to the group | ||
| # If group_by_property is not specified but the T.Label exists the assign it to the group | ||
| # If neither is true then do not assign group | ||
| for p in properties: |
There was a problem hiding this comment.
Looks like this loop is not going to be used for anything, can we remove it?
| # If neither is true then do not assign group | ||
| for p in properties: | ||
| logger.debug(p) | ||
| if self.group_by_property: |
There was a problem hiding this comment.
I think we can rework this now to make use of other suggestions. Something like this should work:
group = self.group_by_property if self.group_by_property in properties else ''
I think this would cover all three because the default value for self.group_by_property would now be T_LABEL. And if that default isn't in our properties, we fall back to an empty string. So we're effectively wrapping the first two conditions into one after amending the defaults.
| data = {'title': title, 'label': label} | ||
| data = {'title': title, 'label': label, 'group': ''} | ||
|
|
||
| logger.debug(data) |
There was a problem hiding this comment.
Is this needed? These will be big dumps even for debug logs
…ng up the code to simplify the group assignment and adjusting the switch to better match the standard. This required updating the unit tests as well
| groupby=None | ||
| if args.group_by and args.group_by != '': | ||
| groupby=args.group_by | ||
| logger.debug(f'groupby: {groupby}') | ||
| gn = GremlinNetwork(group_by_property=groupby) |
There was a problem hiding this comment.
the first statement is likely not needed, since we have set a default for --group-by in the argument parser, it will always be present (and default to T.label)
This should just be able to be reduced to:
logger.debug(f'group_by: {args.group_by}')
gn = GremlinNetwork(group_by_property=args.group_by)
…Network and bumped versions
… search as well as added option to ignore grouping
2 participants
Issue #, if available: #11
Description of changes:
Added functionality to allow for grouping/coloring by property for Gremlin results.
Added a switch --groupby or -g followed by the property name to group by e.g. --groupby region . If a vertex does not have that property it will be added to the default group
If no switch is provided it will use the T.label if available. If it's not, then everything is in the default group
The colors/icons associated with these groups can then be set using the standard visjs options: https://visjs.github.io/vis-network/docs/network/groups.html
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.