Improve rendering of ColumnGroup

[Allex-Nik]

Fixes #1245

Swapping is AnyCol with is AnyFrame allows to avoid applying dataFrameOf() to a ColumnGroup object when convertToDataFrame() in KotlinNotebookPluginUtils is called.

In that case, a separate ColumnGroup object is rendered without the group,

which might look a bit unusual, but the reason to implement it is to ensure more intuitive access to the columns (because we can only access columns a and b and not letters from the code; with the existing approach we would see the group letters, but would not be able to access it).

Cast asDataFrame will leave it without the group being rendered:

At the same time, if we convert this ColumnGroup object to a dataframe (with the function public fun AnyBaseCol.toDataFrame(): AnyFrame = dataFrameOf(listOf(this)), which is already present in the library), the group is rendered in the dataframe:

This seems to be a correct result because in all cases rendering of the columns conforms to the way we access columns from the code (so the structure of data is reflected correctly), and column groups inside a dataframe are actually shown as groups, which prevents the user from being misled.

[Jolanrensen]

Hmm, while the results are a bit more consistent, it's not exactly how I imagined it to look, and there can still be different results depending on whether you use toDataFrame() and asDataFrame(), which can be confusing.

What I'd expect when working with a column group:

and when working with a DataFrame:

wdyt?

This might require some extra checks, as júst instance checks are not enough to figure out the declared type.

[koperagen]

What I'd expect when working with a column group:

For df.columnGroup displaying columnGroup header would be redundant because we refer to column as group.a, group.b, not group.columnGroup.a. So there's a consistency between rendered structure and actual structure + ColumnGroup fulfills its DataFrame contract. I'd go with this fix

[Jolanrensen]

For df.columnGroup displaying columnGroup header would be redundant because we refer to column as group.a, group.b, not group.columnGroup.a. So there's a consistency between rendered structure and actual structure + ColumnGroup fulfills its DataFrame contract. I'd go with this fix

I agree with that, but the difference between a column group and a dataframe is that the former can have a name. That is not displayed anywhere in the output currently, but it's still valuable information. So while it could render like a dataframe, the name of the column group should still be visible, I think.

I'm not sure whether we have such an ability in the current table output, but at least, rendering it as a DF with a single column group achieves this goal.

[Allex-Nik]

I agree that displaying the group header for a column group looks more intuitive. At the same time, it seems important that we can access the columns from the code in the same way they are rendered.

I'm not sure whether we have such an ability in the current table output, but at least, rendering it as a DF with a single column group achieves this goal.

I was thinking: would it be correct if we achieved the goal of making the column group visible by applying the existing method toDataFrame() to it? Like here:

In this case we would also be able to see the header of the group if we need it (with the additional operation though), and access to the columns is still consistent. Or would it break the logic?

toDataFrame() and asDataFrame() would still be different with this approach, I see

[Jolanrensen]

Hmm, so you'd want to shadow the declared val columnGroup: ColumnGroup<*> with val columnGroup: DataFrame<T> = columnGroup.toDataFrame()... Hmm. I'm not sure.

It would be a new concept for sure. We usually only change DataFrame<*> -> DataFrame<SomeType>. That said, a ColumnGroup is a subtype of DataFrame, so it's not totally outlandish...

I'm not sure :) We need some more input. @AndreiKingsley @zaleslaw @koperagen what do you think?

[koperagen]

I think original fix is good because it follows liskov substitution wrt to Show table (dataframe) operation. on ColumnGroup instance it gives you a table with its columns without any special code.
Current behavior seems accidental, not how it was originally intended, due to rewrite of Jupyter renderer callbacks to a single when with type checks