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.

Sign up for GitHub

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

Jump to bottom

Add toDataFrame(KClass) overload to convert type erased lists #825

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add toDataFrame(KClass) overload to convert type erased lists #825

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ public inline fun <reified T> Iterable<T>.toDataFrame(): DataFrame<T> =
properties()
}

public fun Iterable<*>.toDataFrame(klass: KClass<*>): DataFrame<*> =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add some small KDocs for new public functions :) Also, maybe people want to specify roots/maxdepth as well, would that be possible here?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, since we supply a KClass<T> we can get the type information. If we have an Iterable<Any?>, we know we'll get a DataFrame<T>.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finally, would it make sense to offer overloads with body: CreateDataFrameDsl<T>.() -> Unit too?

Copy link
Collaborator Author

@koperagen koperagen Aug 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, since we supply a KClass we can get the type information. If we have an Iterable<Any?>, we know we'll get a DataFrame.

If you have KClass with specific T, i'd say you might as well use list.toDataFrame() without KClass?

Finally, would it make sense to offer overloads with body: CreateDataFrameDsl.() -> Unit too?

Well, this overload is CreateDataFrameDsl<T>.() -> Unit = { properties() } so we'd need to combine them. Also it's supposed to be used when only runtime KClass<*> is available. What useful can be done with CreateDataFrameDsl<Any?>.() -> Unit?

Also, maybe people want to specify roots/maxdepth as well, would that be possible here?

roots - no, because Any?. maxDepth - yes

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have KClass with specific T, i'd say you might as well use list.toDataFrame() without KClass?

Hmm, I guess you're right. I first thought it might be possible to use another class as argument of toDataFrame(), then it would make sense to get T... However, that results in java.lang.IllegalArgumentException: object is not an instance of declaring class. It might be worth mentioning in attached KDocs that all elements in the iterable must be exact instances the KClass given.

createDataFrameImpl(klass) {
properties()
}

@Refine
@Interpretable("toDataFrameDsl")
public inline fun <reified T> Iterable<T>.toDataFrame(noinline body: CreateDataFrameDsl<T>.() -> Unit): DataFrame<T> =
Expand Down
10 changes: 10 additions & 0 deletions core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,4 +452,14 @@ class CreateDataFrameTests {
df.participants[0].city
}
}

@Test
fun `convert type erased list to dataframe`() {
val data = listOf(Person("Alice", "Cooper", 15, "London"))
val erased: List<Any?> = data
val df = erased[0]?.let {
erased.toDataFrame(it::class)
}
df shouldBe data.toDataFrame()
}
}
Loading