-
Notifications
You must be signed in to change notification settings - Fork 513
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
Rename Sparkey hash* methods to avoid shadowing regular methods #3894
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with one comment - although I'm curious how @nevillelyh and/or @regadas feel about breaking API compatibility. (This is a brand new feature without much usage yet, so I'm personally okay with it here.)
@@ -30,6 +30,6 @@ class LargeHashSCollectionFunctions[T](private val self: SCollection[T]) { | |||
*/ | |||
def hashFilter(sideInput: SideInput[SparkeySet[T]]): SCollection[T] = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this also change to largeHashFilter
while we're at it? (There is a regular hashFilter
as well, it seems.)
Also it looks like scalafmt is complaining - can you try running |
Codecov Report
@@ Coverage Diff @@
## main #3894 +/- ##
==========================================
- Coverage 62.07% 62.01% -0.06%
==========================================
Files 286 286
Lines 10222 10196 -26
Branches 358 371 +13
==========================================
- Hits 6345 6323 -22
+ Misses 3877 3873 -4
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 we will release this under 0.11.0
33d992b
to
465996a
Compare
…ify#3894) Co-authored-by: Filipe Regadas <[email protected]>
The
hashJoin
,hashLeftOuterJoin
,hashFullOuterJoin
,hashIntersectByKey
,hashSubtractByKey
methods incom.spotify.scio.extra.sparkey.PairLargeHashSCollectionFunctions
were shadowing the corresponding methods incom.spotify.scio.values.PairHashSCollectionFunctions
.As an example, the following test doesn't compile:
This PR contains a breaking change that fixes this issue. I think renaming those (relatively new) methods to
largeHash*
makes more sense, since they all take an input likeSideInput[SparkeySet[K]]
orSideInput[SparkeyMap[K]]
Please let me know if this isn't an approach you want to consider and if you'd like to fix this another way.