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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix image retrieval over urlretrieve + svg import errors #781

Merged
merged 6 commits into from Jun 9, 2019

Conversation

pappasam
Copy link
Contributor

@pappasam pappasam commented Jun 7, 2019

Resolves #780 and a new svg-related issue.

I followed the same six-related pattern found throughout createpdf.py, genpdftext.py, and pdfbuilder.py.

SVG-related issue

SVG image support is currently optional. By importing the svgimage module at the top of the image.py module, any rst document that contained an image relied on having svglib installed, whether or not that image were an svg. This PR fixes that problem. 馃帀

SVG image support is optional. By importing the svgimage module at the
top of the image.py module, any rst document that contained an image
would rely on having svglib installed. This commit fixes that problem.
For Python 3:
  from urllib.request import urlretrieve

For Python 2:
  from urllib import urlretrieve

Now remote images are correctly retrieved.

Resolves rst2pdf#780
@akrabat
Copy link
Member

akrabat commented Jun 8, 2019

The SVGImage import change is causes three tests to fail :(

This reverts commit ab2c81e.

This import statement is monkey-patched by the inkscape extension, which
means we need to import SVGImage at the module level.
Output in test matches reference image look
This preserves monkey patching behavior for inkscape.
@pappasam
Copy link
Contributor Author

pappasam commented Jun 8, 2019

@akrabat I've fixed the tests, sorry about that.

@akrabat akrabat added this to the 0.95 milestone Jun 9, 2019
akrabat added a commit to akrabat/rst2pdf that referenced this pull request Jun 9, 2019
@akrabat akrabat merged commit 44a0b6c into rst2pdf:master Jun 9, 2019
@akrabat
Copy link
Member

akrabat commented Jun 9, 2019

Thanks @pappasam!

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.

urllib.urlretrieve does not exist in Python 3, image directive fails
2 participants