Rename dfs function

[Jolanrensen]

When I first read

df.select { dfsOf<Type> { it.size() > 10 } }

I thought it meant "Select dataframes of given type where their size is larger than 10".
Asking a friend of mine, they recognized "dfs" as "depth first search" but then they assumed it would return the deepest nested column, since a search is done when a result is found.

Plus, since this function traverses all column groups, it could just as well have been named bfs (breadth first search) and give the same result.

Clearly, this function needs renaming to avoid further confusion.

The functions that need to be renamed include dfs {}, dfsOf<> {} and allDfs(includeGroups: Boolean). Plus, in the internal parts of the library, the name DFS is also sometimes used to just mean a "recursive flattening" of columns, which is definitely not a "search".

Candidates for a rename include:

df.select { traversingGroups { it.size() > 10 } }
df.select { traversingGroupsColsOf<Type> { it.size() > 10 } } // ??
df.select { allTraversingGroups(includeGroups = false) } // ?

or

df.select { cols(traversingGroups = true) { it.size() > 10 } }
df.select { colsOf<Type>(traversingGroups = true) { it.size() > 10 } }
df.select { all(traversingGroups = false, includeGroups = true) } // ?

or

df.select { allLevels { it.size() > 10 } }
df.select { allLevelsOf<Type> { it.size() > 10 } } // ?
df.select { allLevels(includeGroups = false) } // ?

Other ideas are welcome :)

[pacher]

Plus many. I thought the same thing, dfs = dataframes so probably something to do with frame columns. And I was always thinking "if only there would be a way to get all the columns recursively from all the nested groups". I was doing some workarounds and planning to try to implement it at some point and make a PR.
Until I finally saw "depth first search" mentioned once somewhere in the docs at the bottom of the page.
But now that I know it, it is hard to beet the name dfs in conciseness. And I use it quite a lot, so every letter counts because of its place inside select dsl. Having traversingGroups everywhere would look awful and be a big bummer for me.

What would help is a dedicated page in the docs visible in the index tree or a more prominent section with the words "traverse" or "recursive". I would certainly remember that such functionality exists when reading the docs for the first time. But at that point dfs was mentioned only once in some table with other functions and I missed it.

Upd.: maybe deep? deepCols? Like deepCopy. Or rec. Even just dp. Something short but compelling the user to check the docs instead of making assumptions.

[Jolanrensen]

deepCols etc. however makes the api less readable out loud which is one of the strengths of the Dataframe DSL.

I also came up with
select withinGroups (but that leaves out the top level), alsoInGroups, andInGroups, inGroupsToo, recursively.

[Jolanrensen]

And if possible
select { cols { it.size() > 10 }.recursively() }

[koperagen]

I think there were variations of this in our Slack discussion, but we never actually considered just allRecursively..?
Regarding docs - thanks for the feedback. I'm now close to adding tables there that should make operations' results more obvious, and then i can also provide some more info on this topic.

[pacher]

What is the difference between select { all() } and select { cols() } ?

My point is that all or allCols() is the perfect name for dfs(). That what I would expect from allXXX functions, recursive behavior. Perhaps we could deprecate and re-purpose all. I know it is a breaking change, but that's why we are at 0.x.x
Then we can also have allGroups() in addition to allCols() etc.

I also find dfs() and allDfs() a little bit confusing. Why can't we just have includeGroups: Boolean parameter in dfs()? Or maybe even just drop allDfs(). Currently if the user want to select with condition, they have to use dfs and include !it.isColumnGroup() in their predicate anyway.

This and if we rename allUntil/Before/After/Since will allow us to use all uniformly across the api to indicate recursive operations.

[Jolanrensen]

@pacher all() is still tricky IMO. I tend to use it a lot like
groupBy { all() } and there you don't expect it to work recursively.

I did have a successful attempt at creating a wrapper around ColumnSet that holds the original column set and query used to create it. This means that colsOf<Type> { something }.recursively() is now a viable option :)
Also select { all().recursively(includeGroups = false) }

[Jolanrensen]

Working on it in https://github.com/Kotlin/dataframe/tree/dfs-rename
The basics work, along with @Deprecation annotations. However, some tests seem to fail due to changed orders.

[pacher]

How about going one level higher? I mean, we could have different top-level functions, like selectAll {} or deepSelect {} or even selectRecursively {}, although I really don't like the last one. The point is to change the meaning of the functions depending on the context, like select { all() } will select all top level but deepSelect { all() } or select { recursive { all() } } to select all recursively. We can have a separate DSL for deep selections.

Just brainstorming, did not use it enough yet to really get an idea of what is better.

But I wanted to say that I do really like and fully embrace the hierarchical nature of kdf. So almost all of my columns are neatly organized in nested groups and I use recursive selectors most of the time. They do tend to get rather big though and .recursively(includeGroups = false) all over the place scares me a bit. It does look good as an API design, but I am trying to find a more concise way.

I am already used to dfs and it is very short. Do we need to remove it or can we keep it for "backwards compatibility" ;-) and have both?

[Jolanrensen]

Hmm, a level higher will probably be a bit too much, plus, since we have so many functions, hard to maintain.
We could probably also make an overload for .rec() or something short if it's used often, but probably dfs will have to go. It won't be immediate of course :) @Deprecated with level Warning and ReplaceWith etc.

[pacher]

Currently it is easy to write own recursive helper extensions, for example

fun <T> DataFrame<T>.deepSelectGroups(predicate: (ColumnGroup<*>) -> Boolean) =
    select { dfs { it.isColumnGroup() && predicate(it.asColumnGroup()) } }

I hope it will be the same easy with your new query wrappers when they are finished.

[Jolanrensen]

Maybe we should also consider #270

[nikitinas]

I vote for .recursively() modificator that can be applied to any columns selector.

select { cols { it.size() > 10 }.recursively() }

[Jolanrensen]

One note for .recursively(): It changes cols {} from taking (AnyCol) -> Boolean to taking (ColumnWithPath<*>) -> Boolean aka ColumnFilter<*>. Otherwise, you won't be able to reason with depth etc.
This doesn't make much difference in the cols {} API, however it does change exact equality, making:
df.group { cols { it != name } }.into { "nameless" } -> df.group { cols { it.data != name } }.into { "nameless" }