Fixed SVG file generation on windows (#93)

[smarie]

This pull request fixes #93.

It basically replaces the use of pipes.Template, that does not work on windows, with use of subprocess.Popen. An alternate code, using a temporary file, that is probably even more portable, is also included and commented out - just in case this version does not work on ALL targets (I expect it to work on both POSIX-compliant and WIN though).

The final terminal prints are also improved so as to display a correct error message in case of failure.

[smarie]

Hi there, any news concerning this PR ?
It is relatively small and straightforward so it should really not take long to review.
Thanks in advance! By the way I also opened a bug so that it works on PyCharm when graphviz is installed using conda: https://youtrack.jetbrains.com/issue/PY-32408

So if the bug is fixed on your side and on their side, windows users will finally be able to generate the svg file directly :)

[eeaston]

Many thanks @smarie, this is really great! Very sorry for the super-slow reply - I'll pop my comments in-line

[eeaston]

Really nice, thanks - just a couple of small tidies and we can merge it in.

[smarie]

Thanks for the review !

[smarie]

It seems that you have an error in your continuous integration system:

INFO:coveralls:{"message":"Couldn't find a repository matching this job.","error":true}

[eeaston]

Hi, can you rebase from master? We've fixed this one now.

[smarie]

I'm not sure what you mean - I'm already on my master branch here. I'll just push a dummy commit

[smarie]

Still same error. Maybe the best is to merge on your side using commandline (github explains how on this page) then push

[smarie]

Any news on this @eeaston ? It would be great to close it asap now - it is quite old already. As noted above if your CI is not working for external PRs, you can always merge the PR into your local environment and then push yourself.

[smarie]

I tried to reproduce the error. It does not seem to come from my changes, but from the fact that pytest-profiling\\tests\\unit\\prof\\combined.prof is not created during the unit test execution. So the svg generation fails. See test_generates_svg (in unit tests) for details.

Maybe this was already the case before but the new method is less tolerant to the file not actually existing ?

I have to admit that I can't make more progress by myself here because I have troubles understanding the structure of your unit test, in particular where the .prof file is supposed to be generated.

[smarie]

Up: could this be merged ? It seems to me that this is a very small issue, but unfortunately I have absolutely no clue how to make your test environment work on my local computer: it seems to use advanced combinations of virtualenv and other things, that simply do not work on my windows config :(

@eeaston , could you try to run to checkout the PR code and run the tests locally to see what is the side-effect that was introduced ?

Thanks a lot for this great plugin, again - this PR would make it so much easier to use on windows !

[malthe]

Bump?

[smarie]

I'd love that :) but unfortunately this particular plugin does not seem to be very much maintained. @eeaston do you have more information on this plugin status and the possibility to fix this important issue ?

[eeaston]

Hey guys, apologies again for the long delays. I've had some time over the holidays to spend on this and am setting up a Windows build on Travis which I've just about got working now. Once I have the tests going for the profiling plugin I'll merge this one in.
Thanks for your patience!

[eeaston]

Windows build is a-go now, I'll merge this in and add it to the build. Sorry again for the long delay!

[smarie]

Great news !!! Thanks so much @eeaston . Cant wait for the new release.