POJO toDataFrame support (and array improvements)

[Jolanrensen]

Fixes #380

This PR relaxes, the toDataFrame() properties() DSL to allow for getter functions too.

If no memberProperties are available, it'll fall back to getter-like memberFunctions.
Also, it now tries to sort by any constructor if there are multiple (common in java with a no-arg constructor).

EDIT:

• Added that toDataFrame { properties } treats arrays as values using new isArray reflection helpers.
• Fixed rendering of Array types in output
• Added array value rendering to output

[zaleslaw]

Add a little bit more work on clarification and refactoring for future generations

[koperagen]

Let's add a check in isValueType for different arrays. Maybe there's a simple check? If there's no, below code from compiler for reference.
Also, functions in java class can in theory be static, we should filter them out.
Please check how Kotlin reflection sees a List and Int, int, Number returned from Java bean: it could be that they're not converted automatically to their Kotlin counterparts and our isValue or isIterable checks won't work.
Last thing that worth checking is type arguments. Map<String, Integer> from Java can become something like (Mutable)Map<String!, Int!> in Kotlin. Expected result here would be Map<String?, Int?>

private fun ConeKotlinType.isArrayType(isNullable: Boolean?): Boolean {
    return isBuiltinType(StandardClassIds.Array, isNullable) ||
            StandardClassIds.primitiveArrayTypeByElementType.values.any { isBuiltinType(it, isNullable) } ||
            StandardClassIds.unsignedArrayTypeByElementType.values.any { isBuiltinType(it, isNullable) }
}

val primitiveTypes = setOf(Boolean, Char, Byte, Short, Int, Long, Float, Double)

    val primitiveArrayTypeByElementType = primitiveTypes.associateWith { id -> id.shortClassName.primitiveArrayId() }

 val unsignedTypes = setOf(UByte, UShort, UInt, ULong)
    val unsignedArrayTypeByElementType = unsignedTypes.associateWith { id -> id.shortClassName.primitiveArrayId() }

private fun Name.primitiveArrayId() = ClassId(StandardClassIds.Array.packageFqName, Name.identifier(identifier + StandardClassIds.Array.shortClassName.identifier))

[koperagen]

Let's filter out functions that have type arguments too

class B {
    fun <T> getA(): T {
        return TODO()
    }

    fun <T> getAa(): List<T> {
        return TODO()
    }
}

fun t(b: B) {
    b.getAa<Int>()
}

[Jolanrensen]

Let's add a check in isValueType for different arrays. Maybe there's a simple check? If there's no, below code from compiler for reference. Also, functions in java class can in theory be static, we should filter them out. Please check how Kotlin reflection sees a List and Int, int, Number returned from Java bean: it could be that they're not converted automatically to their Kotlin counterparts and our isValue or isIterable checks won't work. Last thing that worth checking is type arguments. Map<String, Integer> from Java can become something like (Mutable)Map<String!, Int!> in Kotlin. Expected result here would be Map<String?, Int?>

private fun ConeKotlinType.isArrayType(isNullable: Boolean?): Boolean {
    return isBuiltinType(StandardClassIds.Array, isNullable) ||
            StandardClassIds.primitiveArrayTypeByElementType.values.any { isBuiltinType(it, isNullable) } ||
            StandardClassIds.unsignedArrayTypeByElementType.values.any { isBuiltinType(it, isNullable) }
}

val primitiveTypes = setOf(Boolean, Char, Byte, Short, Int, Long, Float, Double)

    val primitiveArrayTypeByElementType = primitiveTypes.associateWith { id -> id.shortClassName.primitiveArrayId() }

 val unsignedTypes = setOf(UByte, UShort, UInt, ULong)
    val unsignedArrayTypeByElementType = unsignedTypes.associateWith { id -> id.shortClassName.primitiveArrayId() }

private fun Name.primitiveArrayId() = ClassId(StandardClassIds.Array.packageFqName, Name.identifier(identifier + StandardClassIds.Array.shortClassName.identifier))

lucky for us, static functions aren't in memberFunctions so this works by default :) working on the rest...

[Jolanrensen]

I tried to add your requests to the PR and even added a little extra I needed for debugging/testing (see top comment).

I also spawned a couple of new related issues:

• Missing overloads for Iterable<ArrayLike>.toDataFrame() #676
• Type inference of Array<primitive> may be unpredictable #678
• columnOf(arrayOf(1)) type inferred as Array<*> #679

[koperagen]

Let's not introduce comments that are essentially exactly follow the code? Code is easier to read and is always factual, unlike plain text here

[Jolanrensen]

I agree in the same file, but if you're calling the code from another place, all you see is the signature and the KDoc. I don't want to navigate to a piece of code to figure out what it does.

[koperagen]

ctrl+shift+i?

[Jolanrensen]

haha okay that might work, but still, I think there's no such thing as "unnecessary docs". If university programming has taught me anything is that documentation, no matter how useless it may seem to write, will help you in some way in the future.

[koperagen]

Is it really needed? It was reviewed and merged, not sure why it needs to be changed without actual functional changes. I don't like when PR has things like this

[Jolanrensen]

Well, the complexity of this huge function is someting @zaleslaw stuggeled with and I agree. Lowering the cognitive complexity, like here, improves the codebase as a whole. And face it, we're not gonna do a refactor of everything anytime soon, so I think it's okay to do this along with PRs. I will try to do it in separate commits when I can though, so you can more easily skip it in reviews

[koperagen]

Honestly i still don't understand what to expect from this as a user. (regarding secondary constructors etc) I'd like something simpler
Btw, by default reflection probably returns properties / functions in some specific order too?

[Jolanrensen]

nope :/ reflection returns them completely random.

The idea was to come as close to what a user expects the order of columns to be as possible. So if they have a weird class like

class A {
  constructor()
  constructor(foo: Int, bar: Int)
  constructor(bar: Int, too: Int)
  ...
}

well, you expect foo to be before bar and too after it, right? That's what this function achieves. It sorts the columns according to the order of all constructors. If there's a primary constructor, obviously that is used first.

[Jolanrensen]

Now simplified to just sorting lexicographically and by primary/single constructor

[koperagen]

Looks good, thank you!