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

open_filename doesn't accept a pathlib object #491

Closed
estshorter opened this issue Sep 9, 2020 · 1 comment · Fixed by #492
Closed

open_filename doesn't accept a pathlib object #491

estshorter opened this issue Sep 9, 2020 · 1 comment · Fixed by #492

Comments

@estshorter
Copy link
Contributor

Bug report

If the filename argument is a pathlib object, it is mistakenly recognized as a file handler.
I think open_filename should accept a pathlib object.

class open_filename(object):
"""
Context manager that allows opening a filename and closes it on exit,
(just like `open`), but does nothing for file-like objects.
"""
def __init__(self, filename, *args, **kwargs):
if isinstance(filename, str):
self.file_handler = open(filename, *args, **kwargs)
self.closing = True
else:
self.file_handler = filename
self.closing = False

@pietermarsman pietermarsman added this to new in pdfminer.six via automation Sep 12, 2020
@pietermarsman pietermarsman moved this from new to needs more info in pdfminer.six Sep 12, 2020
@pietermarsman pietermarsman moved this from needs more info to needs solution in pdfminer.six Sep 12, 2020
@pietermarsman pietermarsman moved this from needs solution to needs more info in pdfminer.six Sep 12, 2020
@pietermarsman
Copy link
Member

Hi @estshorter,

I was hesitant at first, because there might be a lot of other packages that also do some kind of path representation. And they probably all can map it to a string (which we already support). But since pathlib is a python core package we can support it.

When changing this, it should also be more verbose in case of any expected input. So maybe use isinstance(something, io.IOBase) for the file like output, and add one for the pathlib, and if none match raise an error.

@pietermarsman pietermarsman moved this from needs more info to accepted in pdfminer.six Sep 12, 2020
@pietermarsman pietermarsman moved this from accepted to in progress in pdfminer.six Sep 13, 2020
pdfminer.six automation moved this from in progress to done Sep 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
pdfminer.six
  
done
Development

Successfully merging a pull request may close this issue.

2 participants