search

RNO-G / mattak

RNO-G dataformats
1 stars 1 forks source link

First attempt to allow passing a single (combined) root file to a mat… #23

Closed fschlueter closed 5 months ago

fschlueter commented 8 months ago

…tak::Dataset instead of a directory. Only works for the pyroot backend at the moment.

Addresses #7

fschlueter commented 8 months ago

@cozzyd this is far from being perfect. Depending on whether you pass a directory or single root file you can or can not read additional information which are not within the combined root tree. So it might cause some problem for inexperienced users. Do you know how to fix it, or are you fine with properly documenting it in the doc strings and push it (it is at least better than nothing I would say)

fschlueter commented 7 months ago

@cozzyd can you have a look at it or should I merge it?

cozzyd commented 7 months ago

Oops sorry I missed this, will take a look today!

On November 20, 2023 10:00:17 AM CST, "Felix Schlüter" @.***> wrote:

@cozzyd can you have a look at it or should I merge it?

-- Reply to this email directly or view it on GitHub: https://github.com/RNO-G/mattak/pull/23#issuecomment-1819343401 You are receiving this because you were mentioned.

Message ID: @.***>

cozzyd commented 7 months ago

So at some point, I started doing maybe almost the same thing in the add-dataset-opts branch, but I dont' remember if I finished. See https://github.com/RNO-G/mattak/compare/main...add-dataset-opts

I'll go ahead and try to combine this with that, if there are any differences...

cozzyd commented 7 months ago

I have resurrected the add-dataset-opts branch, which in retrospect was much more work than merging your request due to the nasty merge (that hopefully is alright now...)

I think the add-dataset-opts branch now has the same functionality as this, in addition to some other things:

One thing it doesn't attempt to do is implement the "skip_partial" option when loading a file, which I'm not sure makes sense?

If you are able to test the add-dataset-opts branch that would be great, otherwise I can merge your changes until that branch is "ready" (I'm hesitant to declare that with the minimal testing I've done...)

fschlueter commented 5 months ago

@cozzyd we should agree how to continue with this and the other PR

fschlueter commented 5 months ago

This PR is redundant after the merge of #25