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.

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

"Smart" column add in DynamicDataFrameBuilder #715

Open
AndreiKingsley opened this issue Jun 3, 2024 · 5 comments
Open

"Smart" column add in DynamicDataFrameBuilder #715

AndreiKingsley opened this issue Jun 3, 2024 · 5 comments
Assignees
Labels
enhancement New feature or request good first issue Good issues to pick-up for newcomers
Milestone

Comments

@AndreiKingsley
Copy link
Collaborator

I can't rewrite this code with DynamicDataFrameBuilder:
https://github.com/Kotlin/kandy/blob/2764bce7e9eec4888fdce71b306631cd4e0a208c/kandy-api/src/main/kotlin/org/jetbrains/kotlinx/kandy/dsl/internal/DatasetHandler.kt#L116
Here I want to do the following - if the column I want to add is already in the builder (with the same name and contains the same elements), it shouldn't be added again.

@Jolanrensen
Copy link
Collaborator

Jolanrensen commented Jun 3, 2024

Indeed it might be worth it to add a contains(col): Boolean function to DynamicDataFrameBuilder.

Could you try whether this works?

public operator fun DynamicDataFrameBuilder.contains(col: AnyCol): Boolean =
    toDataFrame().getColumnOrNull(col) == col

Column equality in DataFrame is checked by this function:

internal fun <T> BaseColumn<T>.checkEquals(other: Any?): Boolean {
    if (this === other) return true

    if (this !is AnyCol) return false
    if (other !is AnyCol) return false

    if (name != other.name) return false
    if (type != other.type) return false
    return values.equalsByElement(other.values)
}

So it checks the name and values by default when you use colA == colB

@AndreiKingsley
Copy link
Collaborator Author

I guess it should work, but it's not efficient in terms of performance. So yeah, contains will work I suppose.

@Jolanrensen
Copy link
Collaborator

@AndreiKingsley you can make a PR that adds the function to DynamicDataFrameBuilder if you like :)

@Jolanrensen Jolanrensen added the enhancement New feature or request label Jun 4, 2024
@AndreiKingsley
Copy link
Collaborator Author

Ok

@zaleslaw zaleslaw added this to the 0.14.0 milestone Jul 19, 2024
@zaleslaw zaleslaw added the good first issue Good issues to pick-up for newcomers label Jul 19, 2024
@zaleslaw zaleslaw modified the milestones: 0.14.0, 0.15.0 Sep 16, 2024
@zaleslaw
Copy link
Collaborator

@AndreiKingsley will be great if you have a look at this issue during next release cycle

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good issues to pick-up for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants