rustworkx
rustworkx copied to clipboard
Qiskit
Avoid using temp files when generating Pillow images
Closes #660
This PR switches from creating a temporary file to save the output of the dot command to writing it to a BytesIO file in memory.
This fixes the problematic behaviour of copying Pillow images: they keep the original source pointing to the temporary file when copied. And when we run im.show(), because the file is temporary, it might be already deleted. Hence it is generally better to do the processing in-memory.
@LaurentBergeron I didn't meant to steal this issue from, I was going to send you the problematic code and found the fix.
Can you help me review this though?
Pull Request Test Coverage Report for Build 3183792667
- 0 of 0 changed or added relevant lines in 0 files are covered.
- No unchanged relevant lines lost coverage.
- Overall coverage remained the same at 97.036%
| Totals | |
|---|---|
| Change from base Build 3178193623: | 0.0% |
| Covered Lines: | 13260 |
| Relevant Lines: | 13665 |
💛 - Coveralls
For the record, I tested the example from #660 on a Windows machine and the image displayed correctly without issues. I was also able to build the documentation which contains many calls to graphviz_draw, so I believe this is safe to merge.
I couldn't figure out the details of that new error that I'm getting, but one interesting fact is that the error goes away if I move the contents of the graphviz_draw into my __main__ script. Could it be that the temporary paths are relative to where current code is executed, and not the directory from which the code was first executed? I think there's some more issues that I need to figure out in my rustworx dev environment.
Anyways, I am happy with the new changes, it looks good. I don't want to hold out the PR, although I don't think I have permission to approve?
