-
Notifications
You must be signed in to change notification settings - Fork 37
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
Fixing error with python 3 #14
Conversation
cStringIO is not available in python 3.x.
In python 3.x, BytesIO serve the purpose of ascii based StringIO.
The functions are working for python 3.7 and sqlalchemy 1.3.5. Can we add tox configuration for them? |
You can add the tox configs in this PR, then we also see right away whether the tests pass. |
I have added tox config. All tests are passing. |
Not sure if this is helpful at all, but I was just adapting this code for usage in our project, and had to use BytesIO to make it work. I'm using Python 3.8, sqlalchemy-schemadisplay==1.4.dev0, Pillow==7.0.0, pydot==1.4.1, ImageMagick 6.9.7-4 Q16 x86_64 20170114 |
@nhoening could you give more details on the issue? This PRs tests seem to work fine (which I didn't notice, thanks for updating @mayanksuman !) |
Not sure what you need to know. I made this function (based on what I saw in this PR) in my script, and I had to use This is my function:
Here is the error I see if I use
|
@nhoening have you tried to just use the fork directly? |
No, I already installed 1.4.dev0 (master) for using restricted tables, didn't have time to pin to something more exotic. It works well for me now. If you guys think it's useful, I'll try it out if I ever have 15 minutes. |
Referring to @mayanksuman 's opening comment, where he does mention BytesIO, maybe the tests only cover one kind of io, namely utf-8, which require StringIO? In any case, the error happens for me for both functions (schema and uml graphs) - I'm not expert enough to judge why I need |
@abitrolly I now tried to call this fork's adapted function directly and I get the same error:
Making the same change as I did before (changing On my system,
I can successfully run |
@nhoening |
@mayanksuman while migrating from Travis to GitHub Actions in #28 and #29 I had to do the same code fixes that you did in this PR. Can you see if there is anything left in this PR or it can be closed? And thanks for the contribution anyway. |
@nhoening it is better to reticket your issue that is not covered by tests, so that it can be addressed separately. |
@abitrolly Just checked - I'm still using my own versions of I agree this can be a new ticket once I verified it still isn't working. Do you guys have a version to test this with already that is later than 1.4dev0? |
@nhoening I use this
|
Thanks for the contribution, but I decided to go for the #35 as a fix for this. |
cStringIO is not available in python 3.x.
BytesIO
(for ascii basedStringIO
) andStringIO
(forutf-8
strings IO) is available fromio
library.Fixes #13