Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

merge reorders tree in alphabetical order #44

Closed
DrChainsaw opened this issue Feb 4, 2021 · 3 comments · Fixed by #46
Closed

merge reorders tree in alphabetical order #44

DrChainsaw opened this issue Feb 4, 2021 · 3 comments · Fixed by #46

Comments

@DrChainsaw
Copy link
Collaborator

Maybe this is intended as it does not seem to be stated anywhere that FileTrees shall have any particular order.

julia> tree = maketree("root" => ["b" => ["x"], "a" => ["y"]])
root/
├─ b/
│  └─ x
└─ a/
   └─ y

julia> merge(tree, tree)
root/
├─ a/
│  └─ y
└─ b/
   └─ x

It did surface as a minor inconvenience for me when plotting stuff in the tree after merging with mv as it is a bit hard to control the order in which things appear in the plot; Imagine that a and b have multiple files and I want to plot one series with the values under a and b concatenated.

@shashi
Copy link
Owner

shashi commented Feb 9, 2021

The sorting is to help de-duplication... in general I did not think maintaining order would be important...

what would be a better way to deal with this?

@shashi
Copy link
Owner

shashi commented Feb 9, 2021

I mean what would be a way to keep the original order while allowing fast de-duplication? Maybe we can maintain a vector of clidren and a set of their names too?

@DrChainsaw
Copy link
Collaborator Author

I haven't thought much about it since I wasn't sure if this was intended. Seems nicer if we don't reorganize the children, but maybe it will be a tall promise to make in the long run.

function _combine(cs, combine)
    if !issorted(cs, by=name)
        sort!(cs, by=name)
    end
    i = 0
    prev = nothing
    out = []
    for c in cs
        if prev == name(c)
            out[end] = apply_combine(combine, c, out[end])
        else
            push!(out, c)
        end
        prev = name(c)
    end
    map(identity, out)
end

Is this the place where it happens?

I think this can be rewriten this using a Dict instead of relying on sort! so that out retains the order of cs. Want me to give it a shot?

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 a pull request may close this issue.

2 participants