-
Notifications
You must be signed in to change notification settings - Fork 56
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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) |
There was a problem hiding this comment.
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: |
There was a problem hiding this comment.
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}', |
There was a problem hiding this comment.
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] |
There was a problem hiding this comment.
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: |
There was a problem hiding this comment.
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']) |
There was a problem hiding this comment.
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
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.