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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: Empty string node results in unclear exception #407

Open
rossbar opened this issue May 15, 2022 · 2 comments
Open

BUG: Empty string node results in unclear exception #407

rossbar opened this issue May 15, 2022 · 2 comments

Comments

@rossbar
Copy link
Contributor

rossbar commented May 15, 2022

This was originally reported to NetworkX in networkx/networkx#4834. [Edit: was pointing to #4855]

A minimal reproducing example:

>>> import pygraphviz as pgv
>>> A = pgv.AGraph()
>>> A.add_node("")  # An empty string
Traceback (most recent call last)
   ...
TypeError: decoding to str: need a bytes-like object, NoneType found

As pointed out in the original issue, this may be expected (assuming graphviz doesn't allow empty nodes) but at the very least a more informative exception should be raised.

I think the problem resides around here:

if nh is not None:
n = super().__new__(self, gv.agnameof(nh), graph.encoding)

as gv.agnameof(nh) returns None when nh derives from an empty string. After verifying that graphviz indeed does not allow empty strings as nodes, this should be simple to fix by adding an extra condition or a try/except here.

@dschult
Copy link
Contributor

dschult commented May 15, 2022

Is the PR number referenced in the comment correct? That’s a pr about sphinx.

I do see that agnameof Is defined twice in pygraphviz.i . Once just as calling the function and then a few lines later being defined directly using Python code. The comments suggest this is to handle anonymous edges. I’m not sure of the impact of that change. But it seems that it would be better to use different names for the two functions.

GraphViz itself allows empty strings as labels for nodes. At least the GraphViz playground works just fine with input of a dot file that uses “” as the node’s unique identifier.

@dschult dschult added this to the 1.10 milestone May 16, 2022
@jarrodmillman jarrodmillman modified the milestones: 1.10, 1.11 Aug 20, 2022
@jarrodmillman jarrodmillman modified the milestones: 1.11, 1.12 May 31, 2023
@jarrodmillman jarrodmillman removed this from the 1.12 milestone Jan 4, 2024
@herrjemand
Copy link

Can confirm it is still painfully exists

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants