Skip to content

Conversation

@BenGalewsky
Copy link
Contributor

Problem

There is no way to specify which tree you want an uproot transformer to operate on

Approach

The tree name needs to be appended to the first Call in the AST. It has to be appended along with a filename (which doesn't seem to be used anywhere).

The set_tree operation needs to clone the AST so you can do something like:

q = ds.Select(lambda e: {'el_pt': e['el_pt']})\
    .set_result_format(ResultFormat.parquet)

nominal = q.set_tree("nominal")
mmt_2016 = q.set_tree("mmt_2016")

print(nominal.as_files())
print(mmt_201.as_files())

Cloning the AST was a bit tricky since there is a reference to the FuncADLDataset in a private property, _eds_object.
I wrote a custom __deepcopy__ method to carefully avoid cloning the open file pointer to the cache DB.

I don't think anyone uses _eds_object and we should consider dropping it from the func_adl library

@BenGalewsky BenGalewsky changed the base branch from master to servicex_client September 14, 2023 14:10
@BenGalewsky BenGalewsky force-pushed the set_tree branch 2 times, most recently from 5223d9f to 6f9d171 Compare September 14, 2023 14:29
@gordonwatts
Copy link
Collaborator

I have no idea what the _eds_object is - it might have been used in func_adl_servicex, but that won't matter any longer. So yeah - this is for some technical debt clean up!

@gordonwatts
Copy link
Collaborator

Are there no tests? :-)

Copy link
Collaborator

@gordonwatts gordonwatts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks cool - I guess I'd expect at least one test - these deep copy things are tricky and if we add something to a event data set, this could cause something funny happening downstream.

@BenGalewsky BenGalewsky merged commit 84a4ac4 into servicex_client Sep 14, 2023
@BenGalewsky BenGalewsky deleted the set_tree branch September 14, 2023 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants