uproot4 unable to open ROOT file with colons in the name that uproot3 can

[matthewfeickert]

Hi. I've come across an issue where if a ROOT file has a colon in the name of it uproot3 can open the file but uproot4 fails. I can't give you the file, but I can show you a minimal failing example and then a reproducible example with public files.

Minimal Failing Example

$ tree .
.
├── data-tree
│   └──  data16_13TeV:data16_13TeV.periodA.physics_Main.PhysCont.DAOD_JETM1.grp16_v01_p4061.root
├── issue.py
├── requirements.txt

1 directory, 3 files
$ cat requirements.txt 
uproot
uproot4
$ docker run --rm -it -v $PWD:/data -w /data python:3.8 /bin/bash
root@510598a7f4e8:/data# python -m pip install --upgrade pip setuptools wheel
root@510598a7f4e8:/data# python -m pip install -r requirements.txt
root@510598a7f4e8:/data# python --version
Python 3.8.5
root@510598a7f4e8:/data# python -m pip list
Package        Version
-------------- -------
awkward        0.13.0
cachetools     4.1.1
numpy          1.19.1
pip            20.2.2
setuptools     49.6.0
uproot         3.12.0
uproot-methods 0.7.4
uproot4        0.0.18
wheel          0.35.1
root@510598a7f4e8:/data# cp data-tree/data16_13TeV\:data16_13TeV.periodA.physics_Main.PhysCont.DAOD_JETM1.grp16_v01_p4061.root data-tree/renamed.root
root@510598a7f4e8:/data# python
Python 3.8.5 (default, Aug  5 2020, 08:22:02) 
[GCC 8.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import uproot as uproot3
>>> import uproot4
>>> from pathlib import Path
>>> 
>>> uproot3_file = uproot3.open(
...     "data-tree/data16_13TeV:data16_13TeV.periodA.physics_Main.PhysCont.DAOD_JETM1.grp16_v01_p4061.root"
... )
>>> print(f"uproot3 opens file as {uproot3_file}")
uproot3 opens file as <ROOTDirectory b'/home/feickert/workarea/submitDir/data-tree//data16_13TeV:data16_13TeV.periodA.physics_Main.PhysCont.DAOD_JETM1.grp16_v01_p4061.root' at 0x7fce6da5b5e0>
>>> 
>>> # uproot4 fails with the ':' in the filename
>>> uproot4.open(
...     "data-tree/data16_13TeV:data16_13TeV.periodA.physics_Main.PhysCont.DAOD_JETM1.grp16_v01_p4061.root"
... )
Traceback (most recent call last):
  File "/usr/local/lib/python3.8/site-packages/uproot4/source/file.py", line 74, in __init__
    self._file = numpy.memmap(self._file_path, dtype=self._dtype, mode="r")
  File "/usr/local/lib/python3.8/site-packages/numpy/core/memmap.py", line 225, in __new__
    f_ctx = open(os_fspath(filename), ('r' if mode == 'c' else mode)+'b')
FileNotFoundError: [Errno 2] No such file or directory: 'data-tree/data16_13TeV'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.8/site-packages/uproot4/reading.py", line 78, in open
    file = ReadOnlyFile(
  File "/usr/local/lib/python3.8/site-packages/uproot4/reading.py", line 265, in __init__
    self._source = Source(file_path, **self._options)
  File "/usr/local/lib/python3.8/site-packages/uproot4/source/file.py", line 80, in __init__
    self._fallback = uproot4.source.file.FileSource(file_path, opts)
AttributeError: module 'uproot4.source.file' has no attribute 'FileSource'
>>> # even if that is inside a pathlib object
>>> uproot4.open(
...     Path(
...         "data-tree/data16_13TeV:data16_13TeV.periodA.physics_Main.PhysCont.DAOD_JETM1.grp16_v01_p4061.root"
...     )
... )
Traceback (most recent call last):
  File "/usr/local/lib/python3.8/site-packages/uproot4/source/file.py", line 74, in __init__
    self._file = numpy.memmap(self._file_path, dtype=self._dtype, mode="r")
  File "/usr/local/lib/python3.8/site-packages/numpy/core/memmap.py", line 225, in __new__
    f_ctx = open(os_fspath(filename), ('r' if mode == 'c' else mode)+'b')
FileNotFoundError: [Errno 2] No such file or directory: 'data-tree/data16_13TeV'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.8/site-packages/uproot4/reading.py", line 78, in open
    file = ReadOnlyFile(
  File "/usr/local/lib/python3.8/site-packages/uproot4/reading.py", line 265, in __init__
    self._source = Source(file_path, **self._options)
  File "/usr/local/lib/python3.8/site-packages/uproot4/source/file.py", line 80, in __init__
    self._fallback = uproot4.source.file.FileSource(file_path, opts)
AttributeError: module 'uproot4.source.file' has no attribute 'FileSource'
>>> # but the file itself is fine
>>> uproot4.open("data-tree/renamed.root")
<ReadOnlyDirectory '/' at 0x7fce725e7670>

Failing Reproducible Example

# issue.py
import uproot as uproot3
import uproot4
from pathlib import Path


def main():
    # curl -sL https://github.com/scikit-hep/scikit-hep-testdata/raw/master/src/skhep_testdata/data/uproot-HZZ-lz4.root -o uproot-HZZ-lz4.root
    uproot3.open("uproot-HZZ-lz4.root")
    uproot3.open("uproot:HZZ-lz4.root")
    uproot3.open(Path("uproot:HZZ-lz4.root"))

    uproot4.open("uproot-HZZ-lz4.root")
    uproot4.open("uproot:HZZ-lz4.root")


if __name__ == "__main__":
    main()

root@510598a7f4e8:/data# curl -sL https://github.com/scikit-hep/scikit-hep-testdata/raw/master/src/skhep_testdata/data/uproot-HZZ-lz4.root -o uproot-HZZ-lz4.root
root@510598a7f4e8:/data# cp uproot-HZZ-lz4.root uproot:HZZ-lz4.root 
root@510598a7f4e8:/data# python issue.py 
Traceback (most recent call last):
  File "issue.py", line 17, in <module>
    main()
  File "issue.py", line 9, in main
    uproot3.open("uproot:HZZ-lz4.root")
  File "/usr/local/lib/python3.8/site-packages/uproot/rootio.py", line 63, in open
    raise ValueError("URI scheme not recognized: {0}".format(path))
ValueError: URI scheme not recognized: uproot:HZZ-lz4.root

Comments

I realize that this is probably because uproot4's open's path is

https://github.com/scikit-hep/uproot4/blob/b8828069b9ae52c742fb704f07e1a4e179fe7b30/uproot4/reading.py#L43-L46

and it isn't a good idea to have a file with a colon in it in general. However, there are ATLAS files that do, and it would be nice if uproot3 behavior toward filenames could still be supported. If support for this is firmly out of scope it would be great if there could be some huge warning about this in the docs.

[tamasgal]

Btw. a minimal working example is also just

>>> import uproot4
>>> uproot4.open("foo:bar.root")

The problem is this line https://github.com/scikit-hep/uproot4/blob/b8828069b9ae52c742fb704f07e1a4e179fe7b30/uproot4/_util.py#L157

This simply assumes that whenever there is a colon, the first part is the path and the second part is the object path, which of course fails when there is a colon in the filename.

EDIT:
I played around with the test suite and found for example the pattern file:/path/to/file : object path as seen here https://github.com/scikit-hep/uproot4/blob/b8828069b9ae52c742fb704f07e1a4e179fe7b30/tests/test_0016-interpretations.py#L84

This would be another case to cover inside file_object_path_split(). I am not sure if this pattern is used in the wild, but I think restricting it to file:// would probably be the better choice. But that's a minor issue.

Anyways, the file_object_path_split() needs to be tweaked to cover all the cases when accepting : in filenames and respect the optional object path.

If the object path is always specified with leading and trailing spaces (" : "), then I think it's fairly easy.

[tamasgal]

Funnily this tiny change let's quite a few tests pass but it obviously fails when the object path is appended without leading and trailing spaces around the :

    try:
        index = path.rindex(" : ")
    except ValueError:
        return path, None

Unfortunately there is an ambiguity here, unless we assume that every root file ends with .root.

[tamasgal]

I created a pull request (#80) which however comes with a grain of salt, so let's discuss further.

[jpivarski]

I was going to say that there's an option that simply turns off the colon-splitting, but I look now and I can't find it. I'm sure it was there. Anyway, that was the intention: that a boolean parameter, by default true, would specify whether the filename gets split into filesystem/URL path and in-ROOT party or not.

Also, the splitting character used to be a vertical bar | because this is a very implausible character for a filename, given the amount of trouble it would cause in the shell. The reason I changed it was because it's not a natural separator—nothing about it suggests that this should be the dividing line between paths. By contrast, a colon is used this way (in shell variable lists, such as PATH).

I switched to a colon because it's an illegal character for filenames in MacOS and (if I remember right) URLs. Unix filenames can have almost any character in them, but considering the use of colons in file lists like PATH, that seemed implausible. It also seemed unlikely in Windows due to drive letter confusion.

So the colon had the right guessable meaning and didn't seem like it could be common in real file names. But ATLAS file names have colons in them? That's an important case. Maybe we should switch back to vertical bars?

At the very least, there should be a boolean parameter that just turns off the splitting. The uproot4.open function is already documented, so in principle it's discoverable, and anyone who doesn't discover it can be told pretty quickly, "set split_path_with_bar=False." I'm surprised it's not there because I coulda sworn I added that. It's a two line change (adding the parameter default and the if statement).

On assuming ROOT files always end with ".root", that's a stricter retirement than not including colons. I'd rather not make that assumption.

[tamasgal]

Ah OK, with that additional option it's a bit better but still may cause confusion. I think switching to | is better since sticking to : and the current implementation means that at least in case of ATLAS files one could not load an object path via open(). Or do you consider adding an additional object_path= parameter?

Btw. : in filenames work on macOS (it was an illegal character on class Mac OS and on the old carbon layer of Mac OS).

[jpivarski]

Another possibility is to require paths-in-file to start with a slash (because they are absolute, after all), and then the real separator would be ":/", which works for ATLAS.

Or to only split with a colon (a convenience) if the filename ends with ".root", while not requiring filenames to always end with ".root". Sorry if I misunderstood that that was your actual suggestion.

It drop it entirely. This is an elaboration on the idea of deep getitem (in which you can write a single path that selects through the TDirectory and TBranch hierarchy), but maybe it's an overextension? Maybe it's not bad to require those extra quotes and braces. They were intended for quick navigation.

[jpivarski]

I've been thinking about it all morning. An object_path parameter would defeat the purpose: it's easier to type

uproot4.open("filename.root")["pathname"]

than it is to type

uproot4.open("filename.root", object_path="pathname")

While the file_object["dir1/dir2/branch1/branch2"] feature was requested, the uproot4.open("filename.root:dir1/dir2/branch1/branch2") feature was not requested—it was an extrapolation of that request.

Since there isn't a compelling reason to do otherwise, we should do the simplest thing of not having any colon/vertical bar syntax at all, since that would be the most guessable and wouldn't require reference to the documentation before getting started. Uproot 4 is still in a fluid enough state (no documentation yet) that I can remove something like that.

[tamasgal]

I fully agree ;) I first thought that the column-syntax has also some special shortcuts to get to the data source even more efficiently but looking at the code it seems to me that it's just an alternative to open()[]... but correct me if I am wrong!

[jpivarski]

That's right: it's only syntactic sugar. The same is true of file_object["dir1/dir2/branch1/branch2"], but that syntactic sugar was helpful because people were stumbling over file_object["dir1"]["dir2"]["branch1"]["branch2"]. I guess it's okay if the number of square brackets is reduce to one, rather than being reduced to zero.

[jpivarski]

Now that I'm going in and deleting it, I find that it really is important: interfaces that take TTrees from multiple files need a way to express both succinctly.

lazyarray = uproot4.lazy(["batch1/*.root : treename1", "batch2/*.root : treename2"])

for arrays in uproot4.iterate(["batch1/*.root : treename1", "batch2/*.root : treename2"]):
    ...

Without this, the object path could be a separate parameter (as it was in Uproot3, which was a stumbling block because uproot.iterate("batch1/*.root", "treename1") led people to forget the tree name) but then it can't be different for different files, as above. Allowing it to be different for different files would require 2-tuples or something, if there isn't a syntactic delimiter.

So it is more important than quick and easy typing in the uproot4.open case... I have to think more about it... I'm open to suggestions.

[jpivarski]

Maybe it's staring me in the face: the colon could be outside of quotation marks if the file name specifier is allowed to be a dict:

lazyarray = uproot4.lazy([{"batch1/*.root": "treename1"}, {"batch2/*.root": "treename2"])

but for the common case of all files having the same name, requiring people to know this weird syntax should be unnecessary:

lazyarray = uproot4.lazy(["batch1/*.root", "batch2/*.root"], "treename")

or maybe pick the only TTree if the file has only one TTree (not counting cycle number).