Predicate join operation #434

koperagen · 2023-08-01T10:54:59Z

Main purpose: joining data on operations other than equals

Jolanrensen · 2023-08-01T18:40:37Z

I see you also did some stuff with HTML but I don't really understand what you did there. It seems to have added some extra spacing to each snippet file. Not too important but relevant enough to mention

Jolanrensen · 2023-08-01T18:18:10Z

core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/join.kt

@@ -10,7 +10,9 @@ public fun <A, B> DataFrame<A>.join(
    other: DataFrame<B>,
    type: JoinType = JoinType.Inner,
    selector: JoinColumnsSelector<A, B>? = null
-): DataFrame<A> = joinImpl(other, type, true, selector)
+): DataFrame<A> {
+    return joinImpl(other, type, addNewColumns = type.addNewColumns, selector)


probably provide name for each argument in the function, it looks odd like this :). Also, I'm fine with it being an expression function if it's just a one line thing

wouldn't it look odd too? plus, it's not even me who wrote this initially :D

joinImpl( other = other, type = type, addNewColumns = type.addNewColumns, selector = selector )

no not at all XD I write functions always like that. IMO positional arguments are a nightmare for refactoring and I think originally they were only allowed áfter the positional ones so mixing them looks extra odd to me.

core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/join.kt

Jolanrensen · 2023-08-01T18:20:53Z

core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/predicateJoin.kt

+    type: JoinType = JoinType.Inner,
+    joinExpression: JoinExpression<A, B>
+): DataFrame<A> {
+    return predicateJoinImpl(right, type, addNewColumns = type.addNewColumns, joinExpression)


same as in join, I'd make it an expression function and provide named arguments for all

Jolanrensen · 2023-08-01T18:21:46Z

core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/predicateJoin.kt

+public fun <A, B> DataFrame<A>.filterPredicateJoin(
+    right: DataFrame<B>,
+    joinExpression: JoinExpression<A, B>
+): DataFrame<A> = predicateJoinImpl(right, JoinType.Inner, addNewColumns = false, joinExpression)


argument names :)

Jolanrensen · 2023-08-01T18:22:33Z

core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/predicateJoin.kt

+public fun <A, B> DataFrame<A>.predicateJoin(
+    right: DataFrame<B>,
+    type: JoinType = JoinType.Inner,
+    joinExpression: JoinExpression<A, B>


trailing commas everywhere please :)

Jolanrensen · 2023-08-01T18:34:08Z

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

+    override val right: DataRow<B> = DataRowImpl(index1, rightOwner)
+}
+
+internal fun <A, B> DataFrame<A>.predicateJoinImpl(


comments would be nice XD, bit hard to check it like this

core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/testSets/person/PredicateJoinTests.kt

docs/StardustDocs/topics/operations.md

Jolanrensen · 2023-08-01T18:43:59Z

docs/StardustDocs/topics/predicateJoin.md

+
+Joins two [`DataFrames`](DataFrame.md) by a join expression. 
+
+```kotlin


code block contains non-kotlin code, so the highlighting goes wrong

I think these blocks use pseudocode intentionally everywhere we have them. So should be ok to use here as well. As for highlighting, with kotlin at least valid parts are highlighted. Like interface :) There are no errors on the website that you see in the preview, if you're talking about it

you could also split it up in multiple blocks if you want pseudocode :)
Or do like I usually do:

// some pseudo code () -> Something fun kotlinCode() {}

i don't think that will work, you can see that even inside kotlin code there are symbols like [ ] for optional parts or | for enumeration, etc

Jolanrensen · 2023-08-01T18:44:38Z

docs/StardustDocs/topics/predicateJoin.md

+}
+```
+
+Unlike [join](join.md) similar to one from relational databases, where relations between tables are based on IDs and usually equality between values, predicate join allows any operations that returns Boolean value. That gives you the ability to join data based on various logical operations and heuristics. This can also be useful when dealing with data from disparate sources that are connected, but lack common identifiers or structured relations typical of SQL tables with indices.


please fold the line so I don't have to scroll horizontally to read it, haha

also this sentence should probably be broken up into multiple and have some extra commas for readability.

i rewrote it completely in the end

much better :) Just some small notes above

Jolanrensen · 2023-08-02T11:41:43Z

docs/StardustDocs/topics/join.md

@@ -94,6 +94,7 @@ df.join(other)

 Supported join types:
 * `Inner` (default) — only matched rows from left and right [`DataFrames`](DataFrame.md)
+* `Filter` — only matched rows from left [`DataFrame`](DataFrame.md)


probably add filterJoin below to the joinSpecials as well

Jolanrensen · 2023-08-02T12:09:40Z

the name predicateJoin might actually be a bit long and cumbersome, especially if expanded like innerPredicateJoin. Maybe it could be shortened like joinWhere or even better, be combined with join:

campaigns.innerJoin(visits).where {
    right.date in startDate..endDate
}

or in the join dsl:

campaigns.innerJoin(visits) {
    where { right.date in startDate..endDate }
    // or
    withPredicate {}
}

docs/StardustDocs/topics/predicateJoin.md

koperagen · 2023-08-02T12:34:36Z

the name predicateJoin might actually be a bit long and cumbersome, especially if expanded like innerPredicateJoin. Maybe it could be shortened like joinWhere or even better, be combined with join:
campaigns.innerJoin(visits).where {
    right.date in startDate..endDate
}
or in the join dsl:
campaigns.innerJoin(visits) {
    where { right.date in startDate..endDate }
    // or
    withPredicate {}
}

I agree that predicateInnerJoin is too long. I'd prefer to have original join renamed to joinBy and have this to be named join. Another option i can suggest is joinWith. Combining this operation with column join is a stretch in my opinion. Columnar join has default join statement (columns with the same names from both dataframes). So, those 2 samples will have very different meaning

 campaigns.innerJoin(visits).where {
     right.date in startDate..endDate
 }

 campaigns.innerJoin(visits)

koperagen · 2023-08-02T12:40:10Z

I see you also did some stuff with HTML but I don't really understand what you did there. It seems to have added some extra spacing to each snippet file. Not too important but relevant enough to mention

Yes, spacing changed because it changed in Kotlin sources. trimMargin didn't work because String inserted with interpolation doesn't have any spacing, so common spacing is 0. Maybe i'll find another way to build HTML files

docs/StardustDocs/topics/joinWith.md

Jolanrensen · 2023-08-09T08:38:06Z

docs/StardustDocs/topics/predicateJoin.md

+}
+```
+
+Unlike [join](join.md) similar to one from relational databases, where relations between tables are based on IDs and usually equality between values, predicate join allows any operations that returns Boolean value. That gives you the ability to join data based on various logical operations and heuristics. This can also be useful when dealing with data from disparate sources that are connected, but lack common identifiers or structured relations typical of SQL tables with indices.


much better :) Just some small notes above

koperagen · 2023-08-09T12:13:00Z

@Jolanrensen Thank you!

koperagen added 3 commits July 28, 2023 20:51

make footer optional

9b60e96

optimize allocations according to inspections

5d181cf

add missing DataRow operations

3f824f6

koperagen force-pushed the predicate-join branch from 2a7be39 to 24c1749 Compare August 1, 2023 11:26

koperagen mentioned this pull request Aug 1, 2023

Docs update & DynamicDataFrameBuilder #435

Closed

Jolanrensen self-requested a review August 1, 2023 18:07

Jolanrensen reviewed Aug 1, 2023

View reviewed changes

koperagen force-pushed the predicate-join branch from 132a583 to 60c5da8 Compare August 2, 2023 11:13

Jolanrensen reviewed Aug 2, 2023

View reviewed changes

docs/StardustDocs/topics/predicateJoin.md Outdated Show resolved Hide resolved

koperagen force-pushed the predicate-join branch from 60c5da8 to 515d3fc Compare August 4, 2023 15:23

koperagen added 6 commits August 8, 2023 14:46

ensure all join overloads work as join with respective supplied JoinType

94fb908

Add predicateJoin operation

2772a50

update snippets due to indentation change in the source code

983958d

save html outputs in PredicateJoin.kt

7abdded

explain difference between join operations

8bb43c1

refactoring: predicateJoin -> joinWith

e8483ee

koperagen force-pushed the predicate-join branch from 515d3fc to e8483ee Compare August 8, 2023 18:08

Jolanrensen reviewed Aug 9, 2023

View reviewed changes

review fixes

6816cba

koperagen merged commit 30c4b09 into master Aug 9, 2023

Jolanrensen deleted the predicate-join branch August 9, 2023 12:49

koperagen mentioned this pull request Aug 11, 2023

Docs update & DynamicDataFrameBuilder #441

Merged

zaleslaw added this to the 0.12.0 milestone Oct 9, 2023


		Joins two [`DataFrames`](DataFrame.md) by a join expression.

		```kotlin

Predicate join operation #434

Predicate join operation #434

Uh oh!

Conversation

koperagen commented Aug 1, 2023

Uh oh!

Jolanrensen commented Aug 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

koperagen Aug 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jolanrensen Aug 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jolanrensen Aug 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jolanrensen commented Aug 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

koperagen commented Aug 2, 2023

Uh oh!

koperagen commented Aug 2, 2023

Uh oh!

Uh oh!

Uh oh!

Jolanrensen Aug 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

koperagen commented Aug 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Jolanrensen commented Aug 1, 2023 •

edited

Loading

koperagen Aug 4, 2023 •

edited

Loading

Jolanrensen Aug 9, 2023 •

edited

Loading

Jolanrensen Aug 2, 2023 •

edited

Loading

Jolanrensen commented Aug 2, 2023 •

edited

Loading

Jolanrensen Aug 9, 2023 •

edited

Loading

koperagen commented Aug 9, 2023 •

edited

Loading