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

ENH: Added PPTX to PDF/PNG #390

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

Conversation

NAnnamalai
Copy link

@NAnnamalai NAnnamalai commented Apr 1, 2021

Added in code for ppt-pdf/png using subprocess by splitting the extension from target value.

Attaching the link for example usage: python, pptx file.

Screenshots if LibreOffice, ImageMagick (pre requisites) if not installed.

Copy link
Contributor

@sanand0 sanand0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See https://learn.gramener.com/guide/contributing/#test-gramex on how to run test cases.

Please add test cases in tests/test_pptxhandler.py. Add a function under TestPPTXHandler called test_png and test_pdf. Just verify that it creates a non-empty PDF and PNG in the correct directory for now.

prs.save(target)
file_name, file_ext = os.path.splitext(target)
pres_name = f'{file_name}.pptx'
prs.save(pres_name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be prs.save(target) -- what if target has a .PPTX in caps? Just retain the original.

Instead, just extract the file_ext -- convert it to lowercase.

prs.save(pres_name)

# if output is a pdf, use soffice to convert pptx to pdf, remove pptx file
if file_ext in ['.pdf', '.png'] and pres_name:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer if file_ext in {'.pdf', '.png'} -- i.e. set is faster than list to search in.

Nest this under if target:. You don't need to run this code unless target is specified. Then you don't need to check for pres_name

# if output is a pdf, use soffice to convert pptx to pdf, remove pptx file
if file_ext in ['.pdf', '.png'] and pres_name:
abs_path = os.path.split(os.path.abspath(target))[0]
subprocess.call(['soffice', '--headless', '--convert-to', 'pdf', f'{pres_name}',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use subprocess.run instead.


# if output is a pdf, use soffice to convert pptx to pdf, remove pptx file
if file_ext in ['.pdf', '.png'] and pres_name:
abs_path = os.path.split(os.path.abspath(target))[0]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check if soffice is in the path using shutilwhich.which('soffice')

If it's not there, use app_log.error() to report an error.

os.remove(pres_name)

# if output is a png, use the image magick with pdf generated in previous step to convert to png, remove pdf file
if file_ext == '.png' and pdf_name:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pdf_name will not be required.


# if output is a png, use the image magick with pdf generated in previous step to convert to png, remove pdf file
if file_ext == '.png' and pdf_name:
subprocess.call(['magick', 'convert', f'{pdf_name}', f'{file_name}-%d.png'])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check using shutilwhich.which. Use subprocess.run

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.

None yet

2 participants