add compatibility with pathlib.Path to root_io #699
Conversation
python/podio/root_io.py
Outdated
| if isinstance(filenames, list): | ||
| str_filenames = [] | ||
| for f in filenames: | ||
| if isinstance(f, (str, Path)): | ||
| str_filenames.append(str(f)) | ||
| else: | ||
| raise TypeError(f"Invalid filename type: {f} (type: {type(f)})") | ||
| return str_filenames | ||
|
|
||
| raise TypeError(f"Invalid filenames argument: {filenames} (type: {type(filenames)})") |
There was a problem hiding this comment.
Can we not simply do
return [str(e) for e in filenames]and the conversion will raise the TypeError for us in case it is not possible to do it?
There was a problem hiding this comment.
Ok. Do you have any objections to using os.fspath instead of str? It is better for catching wrong types, e.g. if None is passed
There was a problem hiding this comment.
If reader.openFiles swallows a list of os.fspath, I have no objections.
There was a problem hiding this comment.
os.fspath() only returns str or bytes. I am unsure what happens if a bytes path is given to reader.openFiles. fspath is just better at throwing TypeError if the argument is not pathlike compared to str(). https://docs.python.org/3/library/os.html#os.fspath
Victor-Schwan
force-pushed
the
pr_path_compati
branch
from
576f58f
to
cb01730
Compare
This reverts commit df74ff4.
| @@ -3,6 +3,8 @@ | |||
|
|
|||
| from ROOT import gSystem | |||
|
|
|||
| from utils import convert_to_str_paths | |||
There was a problem hiding this comment.
| from utils import convert_to_str_paths | |
| from podio.utils import convert_to_str_paths |
There was a problem hiding this comment.
These should also go to where the rest of podio modules are imported
| @@ -3,6 +3,8 @@ | |||
|
|
|||
| from ROOT import gSystem | |||
|
|
|||
| from utils import convert_to_str_paths | |||
There was a problem hiding this comment.
| from utils import convert_to_str_paths | |
| from podio.utils import convert_to_str_paths |
Also these should be grouped with the rest of the podio imports
BEGINRELEASENOTES
ENDRELEASENOTES