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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pdfconverter: Use file path for image conversion #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

manoedinata
Copy link
Contributor

Instead of appending the image content, we could instead use the file path for img2pdf. This reduces huge memory consumption during image conversion (tested in 1GB machine, process getting killed if using image content).

File path usage is also documented in the library homepage in PyPI [1].

[1] https://pypi.org/project/img2pdf/

Instead of appending the image content, we could instead
use the file path for `img2pdf`. This reduces huge memory consumption
during image conversion (tested in 1GB machine, process getting killed
if using image content).

File path usage is also documented in the library homepage in PyPI [1].

[1] https://pypi.org/project/img2pdf/
@manoedinata
Copy link
Contributor Author

Oh, thanks for making this script! It really helps me to download eBook from Fliphtml5 easily :)

@manoedinata
Copy link
Contributor Author

Oh wait! The PDF conversion is failing on Windows system. Please don't merge it just yet!

@marcorentap
Copy link
Owner

Yeah using the paths would be better, I didn't realize that when I made this tool...

It's not working on Windows probably because windows uses \ instead of / as the path separator. Usually we use os.path.join() which handles the differences between OSes. But since Python 3.4, pathlib was introduced. I find it much more convenient to use.

I think we should use it to create the filepath instead of using the basic string "{0}/{1}.jpg".format(folderName, num)

@marcorentap
Copy link
Owner

Also, since Python 3.6, there's f-strings. I think they are much more readable than str.format():

with open(f"{folderName}.pdf", "wb") as file:
    ...

# Instead of this
with open("{0}.pdf".format(folderName), "wb") as file:
    ...

It would be a good idea to start using this whenever we need formatted strings

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.

2 participants