Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch to code formatter Black #267

Merged
merged 5 commits into from Jun 21, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
@@ -1,5 +1,6 @@
[![Build Status](https://www.travis-ci.com/pydot/pydot.svg?branch=master)](https://www.travis-ci.com/pydot/pydot)
[![PyPI](https://img.shields.io/pypi/v/pydot.svg)](https://pypi.org/project/pydot/)
[![Code style: black](https://img.shields.io/badge/code%20style-black-000000.svg)](https://github.com/psf/black)


About
Expand Down
191 changes: 114 additions & 77 deletions dot_parser.py
Expand Up @@ -12,14 +12,25 @@
import sys

from pyparsing import (
nestedExpr, Literal, CaselessLiteral,
Word, OneOrMore,
nestedExpr,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One change that according to me is not optimal, if we think about "better readability".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Black does this to minimize diffs, but when it results in a lot of lines it becomes disturbing. The alternative would be to wrap it in # fmt: off and # fmt: on pragmas, just like I did for the *_ATTRIBUTES constants at the top of pydot.py. However, I also wanted to keep the number of such exceptions low and thought the problem here (from 8 lines to 19 lines) was not big enough to warrant another exception. Just one of the things we have to accept as the price for switching to black, was my conclusion.

Literal,
CaselessLiteral,
Word,
OneOrMore,
Forward,
Group, Optional, Combine,
restOfLine, cStyleComment, nums, alphanums,
Group,
Optional,
Combine,
restOfLine,
cStyleComment,
nums,
alphanums,
printables,
ParseException, ParseResults, CharsNotIn,
QuotedString)
ParseException,
ParseResults,
CharsNotIn,
QuotedString,
)

import pydot

Expand All @@ -35,15 +46,14 @@


class P_AttrList(object):

def __init__(self, toks):
self.attrs = {}
i = 0

while i < len(toks):
attrname = toks[i]
if i+2 < len(toks) and toks[i+1] == '=':
attrvalue = toks[i+2]
if i + 2 < len(toks) and toks[i + 1] == '=':
attrvalue = toks[i + 2]
i += 3
else:
attrvalue = None
Expand All @@ -56,14 +66,16 @@ def __repr__(self):


class DefaultStatement(P_AttrList):

def __init__(self, default_type, attrs):
self.default_type = default_type
self.attrs = attrs

def __repr__(self):
return "%s(%s, %r)" % (self.__class__.__name__,
self.default_type, self.attrs)
return "%s(%s, %r)" % (
self.__class__.__name__,
self.default_type,
self.attrs,
)


top_graphs = list()
Expand All @@ -75,9 +87,11 @@ def push_top_graph_stmt(str, loc, toks):

for element in toks:

if (isinstance(element, (ParseResults, tuple, list)) and
len(element) == 1 and
isinstance(element[0], str_type)):
if (
isinstance(element, (ParseResults, tuple, list))
and len(element) == 1
and isinstance(element[0], str_type)
):

element = element[0]

Expand Down Expand Up @@ -113,7 +127,8 @@ def push_top_graph_stmt(str, loc, toks):

else:
raise ValueError(
'Unknown element statement: {s}'.format(s=element))
'Unknown element statement: {s}'.format(s=element)
)

for g in top_graphs:
update_parent_graph_hierarchy(g)
Expand All @@ -140,25 +155,29 @@ def update_parent_graph_hierarchy(g, parent_graph=None, level=0):

for key, objs in item_dict[key_name].items():
for obj in objs:
if ('parent_graph' in obj and
obj['parent_graph'].get_parent_graph() == g):
if (
'parent_graph' in obj
and obj['parent_graph'].get_parent_graph() == g
):
if obj['parent_graph'] is g:
pass
else:
obj['parent_graph'].set_parent_graph(parent_graph)

if key_name == 'edges' and len(key) == 2:
for idx, vertex in enumerate(obj['points']):
if isinstance(vertex,
(pydot.Graph,
pydot.Subgraph, pydot.Cluster)):
if isinstance(
vertex,
(pydot.Graph, pydot.Subgraph, pydot.Cluster),
):
vertex.set_parent_graph(parent_graph)
if isinstance(vertex, pydot.frozendict):
if vertex['parent_graph'] is g:
pass
else:
vertex['parent_graph'].set_parent_graph(
parent_graph)
parent_graph
)


def add_defaults(element, defaults):
Expand All @@ -168,8 +187,9 @@ def add_defaults(element, defaults):
d[key] = value


def add_elements(g, toks, defaults_graph=None,
defaults_node=None, defaults_edge=None):
def add_elements(
g, toks, defaults_graph=None, defaults_node=None, defaults_edge=None
):
if defaults_graph is None:
defaults_graph = {}
if defaults_node is None:
Expand Down Expand Up @@ -197,8 +217,9 @@ def add_elements(g, toks, defaults_graph=None,
elif isinstance(element, ParseResults):

for e in element:
add_elements(g, [e], defaults_graph,
defaults_node, defaults_edge)
add_elements(
g, [e], defaults_graph, defaults_node, defaults_edge
)

elif isinstance(element, DefaultStatement):

Expand All @@ -221,15 +242,18 @@ def add_elements(g, toks, defaults_graph=None,
else:
raise ValueError(
'Unknown DefaultStatement: {s}'.format(
s=element.default_type))
s=element.default_type
)
)

elif isinstance(element, P_AttrList):

g.obj_dict['attributes'].update(element.attrs)

else:
raise ValueError(
'Unknown element statement: {s}'.format(s=element))
'Unknown element statement: {s}'.format(s=element)
)


def push_graph_stmt(str, loc, toks):
Expand Down Expand Up @@ -268,8 +292,7 @@ def push_default_stmt(str, loc, toks):
if default_type in ['graph', 'node', 'edge']:
return DefaultStatement(default_type, attrs)
else:
raise ValueError(
'Unknown default statement: {s}'.format(s=toks))
raise ValueError('Unknown default statement: {s}'.format(s=toks))


def push_attr_list(str, loc, toks):
Expand All @@ -290,7 +313,7 @@ def get_port(node):
def do_node_ports(node):
node_port = ''
if len(node) > 1:
node_port = ''.join([str(a)+str(b) for a, b in node[1]])
node_port = ''.join([str(a) + str(b) for a, b in node[1]])

return node_port

Expand All @@ -311,16 +334,16 @@ def push_edge_stmt(str, loc, toks):

if isinstance(toks[2][0], ParseResults):

n_next_list = [[n.get_name(), ] for n in toks[2][0]]
n_next_list = [[n.get_name()] for n in toks[2][0]]
for n_next in [n for n in n_next_list]:
n_next_port = do_node_ports(n_next)
e.append(pydot.Edge(n_prev, n_next[0]+n_next_port, **attrs))
e.append(pydot.Edge(n_prev, n_next[0] + n_next_port, **attrs))

elif isinstance(toks[2][0], pydot.Graph):

e.append(pydot.Edge(n_prev,
pydot.frozendict(toks[2][0].obj_dict),
**attrs))
e.append(
pydot.Edge(n_prev, pydot.frozendict(toks[2][0].obj_dict), **attrs)
)

elif isinstance(toks[2][0], pydot.Node):

Expand All @@ -338,18 +361,21 @@ def push_edge_stmt(str, loc, toks):

for n_next in [n for n in tuple(toks)[2::2]]:

if (isinstance(n_next, P_AttrList) or
not isinstance(n_next[0], str_type)):
if isinstance(n_next, P_AttrList) or not isinstance(
n_next[0], str_type
):
continue

n_next_port = do_node_ports(n_next)
e.append(pydot.Edge(n_prev, n_next[0]+n_next_port, **attrs))
e.append(pydot.Edge(n_prev, n_next[0] + n_next_port, **attrs))

n_prev = n_next[0]+n_next_port
n_prev = n_next[0] + n_next_port
else:
raise Exception(
'Edge target {r} with type {s} unsupported.'.format(
r=toks[2][0], s=type(toks[2][0])))
r=toks[2][0], s=type(toks[2][0])
)
)

return e

Expand Down Expand Up @@ -417,84 +443,95 @@ def parse_html(s, loc, toks):

opener = '<'
closer = '>'
html_text = nestedExpr(
opener, closer, (CharsNotIn(opener + closer))
).setParseAction(parse_html).leaveWhitespace()
html_text = (
nestedExpr(opener, closer, (CharsNotIn(opener + closer)))
.setParseAction(parse_html)
.leaveWhitespace()
)

ID = (identifier | html_text | double_quoted_string |
alphastring_).setName("ID")
ID = (
identifier | html_text | double_quoted_string | alphastring_
).setName("ID")

float_number = Combine(
Optional(minus) +
OneOrMore(Word(nums + "."))
Optional(minus) + OneOrMore(Word(nums + "."))
).setName("float_number")

righthand_id = (float_number | ID).setName("righthand_id")

port_angle = (at + ID).setName("port_angle")

port_location = (
OneOrMore(Group(colon + ID)) |
Group(colon + lparen + ID + comma + ID + rparen)
OneOrMore(Group(colon + ID))
| Group(colon + lparen + ID + comma + ID + rparen)
).setName("port_location")

port = (
Group(port_location + Optional(port_angle)) |
Group(port_angle + Optional(port_location))
Group(port_location + Optional(port_angle))
| Group(port_angle + Optional(port_location))
).setName("port")

node_id = (ID + Optional(port))
node_id = ID + Optional(port)
a_list = OneOrMore(
ID +
Optional(equals + righthand_id) +
Optional(comma.suppress())
ID + Optional(equals + righthand_id) + Optional(comma.suppress())
).setName("a_list")

attr_list = OneOrMore(
lbrack.suppress() +
Optional(a_list) +
rbrack.suppress()
lbrack.suppress() + Optional(a_list) + rbrack.suppress()
).setName("attr_list")

attr_stmt = (Group(graph_ | node_ | edge_) +
attr_list).setName("attr_stmt")
attr_stmt = (Group(graph_ | node_ | edge_) + attr_list).setName(
"attr_stmt"
)

edgeop = (Literal("--") | Literal("->")).setName("edgeop")

stmt_list = Forward()
graph_stmt = Group(
lbrace.suppress() +
Optional(stmt_list) +
rbrace.suppress() +
Optional(semi.suppress())
lbrace.suppress()
+ Optional(stmt_list)
+ rbrace.suppress()
+ Optional(semi.suppress())
).setName("graph_stmt")

edge_point = Forward()

edgeRHS = OneOrMore(edgeop + edge_point)
edge_stmt = edge_point + edgeRHS + Optional(attr_list)

subgraph = Group(
subgraph_ + Optional(ID) + graph_stmt).setName("subgraph")
subgraph = Group(subgraph_ + Optional(ID) + graph_stmt).setName(
"subgraph"
)

edge_point << Group(
subgraph | graph_stmt | node_id).setName('edge_point')
edge_point << Group(subgraph | graph_stmt | node_id).setName(
'edge_point'
)

node_stmt = (
node_id + Optional(attr_list) +
Optional(semi.suppress())).setName("node_stmt")
node_id + Optional(attr_list) + Optional(semi.suppress())
).setName("node_stmt")

assignment = (ID + equals + righthand_id).setName("assignment")
stmt = (assignment | edge_stmt | attr_stmt |
subgraph | graph_stmt | node_stmt).setName("stmt")
stmt = (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar concern, but these cases might be more limited than imports.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Also not big enough to warrant an exception, though.

assignment
| edge_stmt
| attr_stmt
| subgraph
| graph_stmt
| node_stmt
).setName("stmt")
stmt_list << OneOrMore(stmt + Optional(semi.suppress()))

graphparser = OneOrMore(
(Optional(strict_) + Group((graph_ | digraph_)) +
Optional(ID) + graph_stmt).setResultsName("graph"))
(
Optional(strict_)
+ Group((graph_ | digraph_))
+ Optional(ID)
+ graph_stmt
).setResultsName("graph")
)

singleLineComment = Group(
"//" + restOfLine) | Group("#" + restOfLine)
singleLineComment = Group("//" + restOfLine) | Group("#" + restOfLine)

# actions

Expand Down