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

Add rf_local_is_in function #388

Closed
wants to merge 7 commits into from

Conversation

vpipkt
Copy link
Member

@vpipkt vpipkt commented Oct 16, 2019

To dos:

  • tests currently failing. Some kind of type error in encoding or something
  • add function to docs
  • add Python bindings
  • update masking docs to use it.

Signed-off-by: Jason T. Brown <jason@astraea.earth>
@vpipkt vpipkt requested a review from metasim October 16, 2019 18:35
checkDocs("rf_local_is_in")

// tile is 3 by 3 with values, 1 to 9
val df = Seq((byteArrayTile, lit(1), lit(5), lit(10))).toDF("t", "one", "five", "ten")
Copy link
Member

Choose a reason for hiding this comment

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

@vpipkt Change Seq((byteArrayTile, lit(1), lit(5), lit(10))) to Seq((byteArrayTile, 1, 5, 10)) and the java.lang.UnsupportedOperationException: No Encoder found for org.apache.spark.sql.Column error should go away. You only want to use litafter the Dataframe is created.

examples = """
Examples:
> SELECT _FUNC_(tile, array);
..."""
Copy link
Member

Choose a reason for hiding this comment

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

I think this is a case where I'd be helped by an example... I'm pretty sure I understand the use case/behavior now, but, but it took a bit of brow furrowing.

Copy link
Member

Choose a reason for hiding this comment

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

Of course reference.md too, when you get the implementation done.

Copy link
Member

Choose a reason for hiding this comment

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

And release-notes.md

Signed-off-by: Jason T. Brown <jason@astraea.earth>
Signed-off-by: Jason T. Brown <jason@astraea.earth>
…l_is_in

Signed-off-by: Jason T. Brown <jason@astraea.earth>
Signed-off-by: Jason T. Brown <jason@astraea.earth>
@vpipkt vpipkt marked this pull request as ready for review October 21, 2019 15:27
@vpipkt
Copy link
Member Author

vpipkt commented Oct 21, 2019

I am building the docs locally just to make sure the fix of #310 doesn't break anything.

@metasim let me know if docs look good and resolve your questions. We can embed a very short example.

@vpipkt
Copy link
Member Author

vpipkt commented Oct 21, 2019

Local docs build was successful.

Signed-off-by: Jason T. Brown <jason@astraea.earth>
@vpipkt vpipkt closed this Oct 23, 2019
@vpipkt vpipkt deleted the feature/local_is_in branch October 23, 2019 18:28
@vpipkt
Copy link
Member Author

vpipkt commented Oct 23, 2019

See #400 which triggers circle CI docs build so we can see the final docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants