Skip to content

Commit f3cc35c

Browse files
JolanrensenJolanrensen
committed
updating isGetterLike and columnName with kdocs and to be more explicit, updating convertToDataFrame implementation to be clearer too. Testing for different types of primitives from java pojo
1 parent 30b3f77 commit f3cc35c

File tree

4 files changed

+158
-95
lines changed

4 files changed

+158
-95
lines changed

‎core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/Utils.kt

Lines changed: 34 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -340,32 +340,51 @@ internal fun List<String>.joinToCamelCaseString(): String {
340340
.replaceFirstChar { it.lowercaseChar() }
341341
}
342342

343+
/** @include [KCallable.isGetterLike] */
343344
internal fun KFunction<*>.isGetterLike(): Boolean =
344-
(name.startsWith("get") || name.startsWith("is")) && valueParameters.isEmpty()
345+
(name.startsWith("get") || name.startsWith("is")) &&
346+
valueParameters.isEmpty() &&
347+
typeParameters.isEmpty()
345348

349+
/** @include [KCallable.isGetterLike] */
350+
internal fun KProperty<*>.isGetterLike(): Boolean = true
351+
352+
/**
353+
* Returns `true` if this callable is a getter-like function.
354+
*
355+
* A callable is considered getter-like if it is either a property getter,
356+
* or it's a function with no (type) parameters that starts with "get"/"is".
357+
*/
346358
internal fun KCallable<*>.isGetterLike(): Boolean =
347359
when (this) {
348-
is KProperty<*> -> true
360+
is KProperty<*> -> isGetterLike()
349361
is KFunction<*> -> isGetterLike()
350362
else -> false
351363
}
352364

365+
/** @include [KCallable.columnName] */
366+
@PublishedApi
367+
internal val KFunction<*>.columnName: String
368+
get() = findAnnotation<ColumnName>()?.name
369+
?: name
370+
.removePrefix("get")
371+
.removePrefix("is")
372+
.replaceFirstChar { it.lowercase() }
373+
374+
/** @include [KCallable.columnName] */
353375
@PublishedApi
354376
internal val KProperty<*>.columnName: String
355377
get() = findAnnotation<ColumnName>()?.name ?: name
356378

379+
/**
380+
* Returns the column name for this callable.
381+
* If the callable contains the [ColumnName] annotation, its [ColumnName.name] is returned.
382+
* Otherwise, the name of the callable is returned with proper getter-trimming iff it's a [KFunction].
383+
*/
357384
@PublishedApi
358385
internal val KCallable<*>.columnName: String
359-
get() = findAnnotation<ColumnName>()?.name
360-
?: when (this) {
361-
// for defining the column names based on a getter-function, we use the function name minus the get/is prefix
362-
is KFunction<*> ->
363-
name
364-
.removePrefix("get")
365-
.removePrefix("is")
366-
.replaceFirstChar { it.lowercase() }
367-
368-
is KProperty<*> -> this.columnName
369-
370-
else -> name
371-
}
386+
get() = when (this) {
387+
is KFunction<*> -> columnName
388+
is KProperty<*> -> columnName
389+
else -> findAnnotation<ColumnName>()?.name ?: name
390+
}

‎core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/api/toDataFrame.kt

Lines changed: 76 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ import kotlin.reflect.jvm.isAccessible
3737
import kotlin.reflect.jvm.javaField
3838
import kotlin.reflect.typeOf
3939

40-
internal val valueTypes = setOf(
40+
private val valueTypes = setOf(
4141
String::class,
4242
Boolean::class,
4343
kotlin.time.Duration::class,
@@ -186,21 +186,21 @@ internal fun convertToDataFrame(
186186
class ValueClassConverter(val unbox: Method, val box: Method)
187187

188188
val valueClassConverter = (it.returnType.classifier as? KClass<*>)?.let { kClass ->
189-
if (!kClass.isValue) null else {
190-
val constructor = requireNotNull(kClass.primaryConstructor) {
191-
"value class $kClass is expected to have primary constructor, but couldn't obtain it"
192-
}
193-
val parameter = constructor.parameters.singleOrNull()
194-
?: error("conversion of value class $kClass with multiple parameters in constructor is not yet supported")
195-
// there's no need to unwrap if underlying field is nullable
196-
if (parameter.type.isMarkedNullable) return@let null
197-
// box and unbox impl methods are part of binary API of value classes
198-
// https://youtrack.jetbrains.com/issue/KT-50518/Boxing-Unboxing-methods-for-JvmInline-value-classes-should-be-public-accessible
199-
val unbox = kClass.java.getMethod("unbox-impl")
200-
val box = kClass.java.methods.single { it.name == "box-impl" }
201-
val valueClassConverter = ValueClassConverter(unbox, box)
202-
valueClassConverter
189+
if (!kClass.isValue) return@let null
190+
191+
val constructor = requireNotNull(kClass.primaryConstructor) {
192+
"value class $kClass is expected to have primary constructor, but couldn't obtain it"
203193
}
194+
val parameter = constructor.parameters.singleOrNull()
195+
?: error("conversion of value class $kClass with multiple parameters in constructor is not yet supported")
196+
// there's no need to unwrap if underlying field is nullable
197+
if (parameter.type.isMarkedNullable) return@let null
198+
// box and unbox impl methods are part of binary API of value classes
199+
// https://youtrack.jetbrains.com/issue/KT-50518/Boxing-Unboxing-methods-for-JvmInline-value-classes-should-be-public-accessible
200+
val unbox = kClass.java.getMethod("unbox-impl")
201+
val box = kClass.java.methods.single { it.name == "box-impl" }
202+
val valueClassConverter = ValueClassConverter(unbox, box)
203+
valueClassConverter
204204
}
205205
(property as? KProperty<*>)?.javaField?.isAccessible = true
206206
property.isAccessible = true
@@ -245,84 +245,99 @@ internal fun convertToDataFrame(
245245
typeOf<Any>()
246246
}
247247
}
248-
val kclass = (returnType.classifier as KClass<*>)
248+
val kClass = returnType.classifier as KClass<*>
249+
250+
val shouldCreateValueCol = (
251+
maxDepth <= 0 &&
252+
!returnType.shouldBeConvertedToFrameColumn() &&
253+
!returnType.shouldBeConvertedToColumnGroup()
254+
) ||
255+
kClass == Any::class ||
256+
kClass in preserveClasses ||
257+
property in preserveProperties ||
258+
kClass.isValueType
259+
260+
val shouldCreateFrameCol = kClass == DataFrame::class && !nullable
261+
val shouldCreateColumnGroup = kClass == DataRow::class
262+
249263
when {
250264
hasExceptions -> DataColumn.createWithTypeInference(it.columnName, values, nullable)
251265

252-
kclass == Any::class ||
253-
preserveClasses.contains(kclass) ||
254-
preserveProperties.contains(property) ||
255-
(maxDepth <= 0 && !returnType.shouldBeConvertedToFrameColumn() && !returnType.shouldBeConvertedToColumnGroup()) ||
256-
kclass.isValueType ->
266+
shouldCreateValueCol ->
257267
DataColumn.createValueColumn(
258268
name = it.columnName,
259269
values = values,
260270
type = returnType.withNullability(nullable),
261271
)
262272

263-
kclass == DataFrame::class && !nullable ->
273+
shouldCreateFrameCol ->
264274
DataColumn.createFrameColumn(
265275
name = it.columnName,
266276
groups = values as List<AnyFrame>
267277
)
268278

269-
kclass == DataRow::class ->
279+
shouldCreateColumnGroup ->
270280
DataColumn.createColumnGroup(
271281
name = it.columnName,
272282
df = (values as List<AnyRow>).concat(),
273283
)
274284

275-
kclass.isSubclassOf(Iterable::class) -> {
276-
val elementType = returnType.projectUpTo(Iterable::class).arguments.firstOrNull()?.type
277-
if (elementType == null) {
278-
DataColumn.createValueColumn(it.columnName, values, returnType.withNullability(nullable))
279-
} else {
280-
val elementClass = (elementType.classifier as? KClass<*>)
281-
282-
when {
283-
elementClass == null -> {
284-
val listValues = values.map {
285-
(it as? Iterable<*>)?.asList()
286-
}
285+
kClass.isSubclassOf(Iterable::class) ->
286+
when (val elementType = returnType.projectUpTo(Iterable::class).arguments.firstOrNull()?.type) {
287+
null ->
288+
DataColumn.createValueColumn(
289+
name = it.columnName,
290+
values = values,
291+
type = returnType.withNullability(nullable),
292+
)
293+
294+
else -> {
295+
val elementClass = elementType.classifier as? KClass<*>
296+
when {
297+
elementClass == null -> {
298+
val listValues = values.map {
299+
(it as? Iterable<*>)?.asList()
300+
}
287301

288-
DataColumn.createWithTypeInference(it.columnName, listValues)
289-
}
302+
DataColumn.createWithTypeInference(it.columnName, listValues)
303+
}
290304

291-
elementClass.isValueType -> {
292-
val listType = getListType(elementType).withNullability(nullable)
293-
val listValues = values.map {
294-
(it as? Iterable<*>)?.asList()
305+
elementClass.isValueType -> {
306+
val listType = getListType(elementType).withNullability(nullable)
307+
val listValues = values.map {
308+
(it as? Iterable<*>)?.asList()
309+
}
310+
DataColumn.createValueColumn(it.columnName, listValues, listType)
295311
}
296-
DataColumn.createValueColumn(it.columnName, listValues, listType)
297-
}
298312

299-
else -> {
300-
val frames = values.map {
301-
if (it == null) {
302-
DataFrame.empty()
303-
} else {
304-
require(it is Iterable<*>)
305-
convertToDataFrame(
306-
data = it,
307-
clazz = elementClass,
308-
roots = emptyList(),
309-
excludes = excludes,
310-
preserveClasses = preserveClasses,
311-
preserveProperties = preserveProperties,
312-
maxDepth = maxDepth - 1,
313-
)
313+
else -> {
314+
val frames = values.map {
315+
if (it == null) {
316+
DataFrame.empty()
317+
} else {
318+
require(it is Iterable<*>)
319+
convertToDataFrame(
320+
data = it,
321+
clazz = elementClass,
322+
roots = emptyList(),
323+
excludes = excludes,
324+
preserveClasses = preserveClasses,
325+
preserveProperties = preserveProperties,
326+
maxDepth = maxDepth - 1,
327+
)
328+
}
314329
}
330+
DataColumn.createFrameColumn(it.columnName, frames)
315331
}
316-
DataColumn.createFrameColumn(it.columnName, frames)
317332
}
318333
}
319334
}
320-
}
335+
321336

322337
else -> {
323338
val df = convertToDataFrame(
324339
data = values,
325-
clazz = kclass,
340+
clazz = kClass,
326341
roots = emptyList(),
327342
excludes = excludes,
328343
preserveClasses = preserveClasses,

‎core/src/test/java/org/jetbrains/kotlinx/dataframe/api/JavaPojo.java

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,16 @@ public class JavaPojo {
66

77
private int a;
88
private String b;
9+
private Integer c;
10+
private Number d;
911

1012
public JavaPojo() {}
1113

12-
public JavaPojo(String b, int a) {
14+
public JavaPojo(Number d, Integer c, String b, int a) {
1315
this.a = a;
1416
this.b = b;
17+
this.c = c;
18+
this.d = d;
1519
}
1620

1721
public int getA() {
@@ -30,29 +34,46 @@ public void setB(String b) {
3034
this.b = b;
3135
}
3236

37+
public Integer getC() {
38+
return c;
39+
}
40+
41+
public void setC(Integer c) {
42+
this.c = c;
43+
}
44+
45+
public Number getD() {
46+
return d;
47+
}
48+
49+
public void setD(Number d) {
50+
this.d = d;
51+
}
52+
53+
public static int getNot() {
54+
return 1;
55+
}
56+
3357
@Override
3458
public boolean equals(Object o) {
3559
if (this == o) return true;
36-
if (o == null || getClass() != o.getClass()) return false;
37-
38-
JavaPojo testPojo = (JavaPojo) o;
39-
40-
if (a != testPojo.a) return false;
41-
return Objects.equals(b, testPojo.b);
60+
if (!(o instanceof JavaPojo)) return false;
61+
JavaPojo javaPojo = (JavaPojo) o;
62+
return a == javaPojo.a && Objects.equals(b, javaPojo.b) && Objects.equals(c, javaPojo.c) && Objects.equals(d, javaPojo.d);
4263
}
4364

4465
@Override
4566
public int hashCode() {
46-
int result = a;
47-
result = 31 * result + (b != null ? b.hashCode() : 0);
48-
return result;
67+
return Objects.hash(a, b, c, d);
4968
}
5069

5170
@Override
5271
public String toString() {
53-
return "TestPojo{" +
72+
return "JavaPojo{" +
5473
"a=" + a +
5574
", b='" + b + '\'' +
75+
", c=" + c +
76+
", d=" + d +
5677
'}';
5778
}
5879
}

‎core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -370,17 +370,25 @@ class CreateDataFrameTests {
370370
// even though the names b, a, follow the constructor order
371371
listOf(KotlinPojo("bb", 1)).toDataFrame() shouldBe dataFrameOf("b", "a")("bb", 1)
372372

373-
// cannot read java constructor parameter names with reflection, so sort lexigographically
374-
listOf(JavaPojo("bb", 1)).toDataFrame() shouldBe dataFrameOf("a", "b")(1, "bb")
375-
376-
listOf(KotlinPojo("bb", 1)).toDataFrame { properties(KotlinPojo::getA) } shouldBe dataFrameOf("a")(1)
377-
listOf(KotlinPojo("bb", 1)).toDataFrame { properties(KotlinPojo::getB) } shouldBe dataFrameOf("b")("bb")
378-
379-
listOf(JavaPojo("bb", 1)).toDataFrame {
373+
// cannot read java constructor parameter names with reflection, so sort lexicographically
374+
listOf(JavaPojo(2.0, null, "bb", 1)).toDataFrame() shouldBe
375+
dataFrameOf(
376+
DataColumn.createValueColumn("a", listOf(1), typeOf<Int>()),
377+
DataColumn.createValueColumn("b", listOf("bb"), typeOf<String>()),
378+
DataColumn.createValueColumn("c", listOf(null), typeOf<Int?>()),
379+
DataColumn.createValueColumn("d", listOf(2.0), typeOf<Number>()),
380+
)
381+
382+
listOf(KotlinPojo("bb", 1)).toDataFrame { properties(KotlinPojo::getA) } shouldBe
383+
dataFrameOf("a")(1)
384+
listOf(KotlinPojo("bb", 1)).toDataFrame { properties(KotlinPojo::getB) } shouldBe
385+
dataFrameOf("b")("bb").groupBy("").concat()
386+
387+
listOf(JavaPojo(2.0, 3, "bb", 1)).toDataFrame {
380388
properties(JavaPojo::getA)
381389
} shouldBe dataFrameOf("a")(1)
382390

383-
listOf(JavaPojo("bb", 1)).toDataFrame {
391+
listOf(JavaPojo(2.0, 3, "bb", 1)).toDataFrame {
384392
properties(JavaPojo::getB)
385393
} shouldBe dataFrameOf("b")("bb")
386394
}

0 commit comments

Comments
 (0)