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

Fix tgf output #4

Closed
wants to merge 2 commits into from
Closed

Fix tgf output #4

wants to merge 2 commits into from

Conversation

aeijdenberg
Copy link

Using --tgf output format seems to break on trivial examples.

e.g.

a.py:

import b
b.c()

b.py:

def c():
    pass

Results in:

$ ./pyan.py a.py b.py --tgf
Traceback (most recent call last):
  File "./pyan.py", line 849, in <module>
    main()
  File "./pyan.py", line 845, in main
    draw_uses=options.draw_uses)
  File "./pyan.py", line 734, in to_tgf
    i1 = id_map[n]
KeyError: <Node a>

By removing the if n.defined: check on line 713, this succeeds.

(The rest of this diff is just trailing whitespace removal noise)

@Technologicat
Copy link
Contributor

(Now going through the PRs from here for inclusion into the Python 3 fork.)

The problem was that the defined checks were not consistent. The code creating id_map checked n.defined, whereas the code writing the edges didn't. Thus, it was trying to add edges between nodes that didn't have an ID for the .tgf, and crashed.

I solved this the other way, adding a check for n.defined in the code writing the edges. Visualizing only defined nodes and edges between them (like the GraphViz output does) should produce a cleaner graph in case of more complex input files.

On my part, thanks for the report and the test case :)

@aeijdenberg
Copy link
Author

Thanks for the follow-up @Technologicat - I'd forgotten all about this! Glad you've found a fix, please feel free to close this PR out.

@Technologicat Technologicat mentioned this pull request Nov 16, 2017
@davidfraser
Copy link
Owner

Closing as requested

davidfraser pushed a commit that referenced this pull request Feb 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants