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.

Sign up for GitHub

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

Jump to bottom

Convert agraph.py warnings from bytes to strings #95

Closed
wants to merge 1 commit into from
Closed

Convert agraph.py warnings from bytes to strings #95

wants to merge 1 commit into from

Conversation

aleksihakli
Copy link

Warning and error raising behaviour is not correct on Python 3,
where bytes objects cannot be implicitly converted to strings,
and the warnings cause a TypeError crash.

Fixes #94

@coveralls
Copy link

coveralls commented Jun 29, 2016 •
edited
Loading

Coverage Status

Coverage remained the same at 81.075% when pulling 5dc64a4 on Vincit:python3-bytes-fixes into 28b9e9d on pygraphviz:master.

@hagberg
Copy link
Contributor

hagberg commented Jul 2, 2016

That looks like the right fix. We should figure out how to add a test for this.

@coveralls
Copy link

coveralls commented Jul 22, 2016 •
edited
Loading

Coverage Status

Coverage remained the same at 81.075% when pulling 814fa5f on Vincit:python3-bytes-fixes into 28b9e9d on pygraphviz:master.

@coveralls
Copy link

coveralls commented Jul 22, 2016 •
edited
Loading

Coverage Status

Coverage remained the same at 81.075% when pulling 857b9df on Vincit:python3-bytes-fixes into 28b9e9d on pygraphviz:master.

@aleksihakli
Convert agraph.py warnings from bytes to strings
78a2217 
Warning and error raising behaviour is not correct on Python 3,
where bytes objects cannot be implicitly converted to strings,
and the warnings cause a TypeError crash.

This patch adds default decoding for errors and warnings
that happen when _run_prog is used and fixes errors
on Windows when e.g. no default fonts are available.
@coveralls
Copy link

coveralls commented Jul 22, 2016 •
edited
Loading

Coverage Status

Coverage remained the same at 81.075% when pulling 78a2217 on Vincit:python3-bytes-fixes into 28b9e9d on pygraphviz:master.

@aleksihakli
Copy link
Author

I don't see an easy way for testing _run_prog RuntimeWarnings outside of Windows. The specific needed IOErrors might be possible to test by mocking failed data output, but that is outside my expertise. Hence I can't contribute on the tests here.

The .decode functionality should work on Linux and Windows platforms since it is the same as other .decode uses in the same program and file.

I hope we can merge this since currently the program can't be used on Windows on Python 3 and the patch does not regress any functionality.

@aleksihakli aleksihakli reopened this Jul 24, 2016
@coveralls
Copy link

coveralls commented Jul 24, 2016 •
edited
Loading

Coverage Status

Coverage remained the same at 81.075% when pulling 78a2217 on Vincit:python3-bytes-fixes into 28b9e9d on pygraphviz:master.

yeus
yeus reviewed Oct 7, 2016
View reviewed changes
Copy link

@yeus yeus left a comment

Choose a reason for hiding this comment

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

seems to work on my system ... with conda and python 3.5

@hagberg
Copy link
Contributor

hagberg commented Nov 4, 2016

Fixed in 272a113

@hagberg hagberg closed this Nov 4, 2016
@hagberg hagberg mentioned this pull request Nov 4, 2016
Closed
@aleksihakli aleksihakli deleted the python3-bytes-fixes branch November 8, 2016 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yeus yeus yeus left review comments

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@aleksihakli @coveralls @hagberg @yeus