[hadd] add option to filter objects/directories to merge #17612
Conversation
silverweed
force-pushed
the
hadd_fromfile
branch
from
5e8eb76 to
6186346
Compare
Test Results 18 files 18 suites 5d 7h 41m 59s ⏱️ For more details on these failures, see this check. Results for commit ebe491f. ♻️ This comment has been updated with latest results. |
| @@ -606,6 +674,10 @@ int main(int argc, char **argv) | |||
| if (args.fCacheSize) | |||
| Info() << "Using " << cacheSize << "\n"; | |||
|
|
|||
| ////////////////////////////// end flags processing ///////////////////////////////// | |||
|
|
|||
| gSystem->Load("libTreePlayer"); | |||
There was a problem hiding this comment.
Why do we need this library?
There was a problem hiding this comment.
Not sure, it was there already (I just moved it past the flags processing in this PR). Perhaps @pcanal knows it.
There was a problem hiding this comment.
The dependency was originally a explicit link added to "allow support for TTree with a build index" and then was moved to a run-time dependency because the usage is hidden being a layer of indirect and was dropped in some case by the remove unused dependency feature of the linker (or something like that).
The original commit ended with "until hadd can use the autoloader" which is likely the case now.
So it probably can be technically removed.
A simple test would to create a TTree, Create and index, save it to the file, merge the result a couple of times and check the results.
There was a problem hiding this comment.
I guess we want to do this in a separate PR
silverweed
force-pushed
the
hadd_fromfile
branch
from
6186346 to
cf25ca7
Compare
| @@ -34,6 +34,16 @@ def get_argparse(): | |||
| "compression setting after fk (for example 206 when using -fk206)"), action = 'store_true') | |||
| parser.add_argument("-ff", help="The compression level use is the one specified in the first input", action = 'store_true') | |||
| parser.add_argument("-k", help="Skip corrupt or non-existent files, do not exit", action = 'store_true') | |||
| parser.add_argument("-L", help=textwrap.fill( | |||
| "Read the list of objects from FILE and either only merge or skip those objects depending on " | |||
There was a problem hiding this comment.
This is a powerful new features but does not match the title of the PR.
The title talks about filtering which files are being merged together while the feature documented here and implemented below is about filtering which objects and TDirectories within the files are actually merged (but does not affect the list of files opened).
There was a problem hiding this comment.
See tutorials/io/mergeSelective.C
There was a problem hiding this comment.
@pcanal the PR title was indeed wrong; I changed "files" to "objects".
Adding -L and -Ltype flags, which allow passing a file name as a "filter file", containing one object path per line. -Ltype determines the filter mode (SkipListed/OnlyListed).
silverweed
force-pushed
the
hadd_fromfile
branch
from
cf25ca7 to
ebe491f
Compare
silverweed
changed the title
This Pull request:
depends on #17609.
It's a new take on #4615, which adds two flags to hadd:
-L <FILE>: specify a file that contains object or directory names, one per line, to be used as a filter. The filter type is specified by the second flag:-Ltype <OnlyListed|SkipListed>: determines whether the list of object names contained in FILE should be used as a whitelist or a blacklist to know which objects to merge. This internally usesTFileMerger::AddObjectNames, so it has the same restrictions, notably you cannot specify a subobject name such asdir/object. If this is a desired feature, it should be added to TFileMerger directly, in a separate PR.Checklist: