-
Notifications
You must be signed in to change notification settings - Fork 63
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
Allow any ArrowReader implementation to be use for reading Arrow data #627 #628
Conversation
2457bf8
to
67df4a3
Compare
import java.time.LocalDate | ||
import java.time.LocalDateTime | ||
import java.time.ZoneOffset | ||
import java.util.Locale | ||
import java.util.* |
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 turn off * imports in the project. So could you do that for consistency too?
@@ -558,23 +565,26 @@ internal class ArrowKtTest { | |||
} | |||
} | |||
|
|||
@Test | |||
fun testArrowReaderExtension() { | |||
private fun expectedSimpleDataFrame(): AnyFrame{ |
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.
missed a space before "{"
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.
Fixed, weird thing that was not detected by linter during my local build
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.
Yes, our linter is a bit broken :/ #364
7df9251
to
b6ab5ea
Compare
b6ab5ea
to
e85ec1d
Compare
e85ec1d
to
56d27e6
Compare
CI agrees, I merged it, thanks! |
ArrowFileReader part use specific method while ArrowStreamReader part rely only on ArrowReader interface methods. This modification just enables any ArrowReader implementation to be use for reading arrow data