-
Notifications
You must be signed in to change notification settings - Fork 59
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
Return SubArrays when possible for arrow list types #446
Conversation
Codecov Report
@@ Coverage Diff @@
## main #446 +/- ##
==========================================
+ Coverage 87.46% 87.49% +0.03%
==========================================
Files 26 26
Lines 3263 3271 +8
==========================================
+ Hits 2854 2862 +8
Misses 409 409
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
(t1 === t2) || (return false) | ||
tt1 = Base.nonmissingtype(t1) | ||
tt2 = Base.nonmissingtype(t2) | ||
if t1 == t2 || (tt1 <: AbstractVector && tt2 <: AbstractVector && eltype(tt1) == eltype(tt2)) |
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.
This is needed to loosen what we consider "equivalent schemas"; i.e. a column w/ eltype Vector{Int}
is now "equal" to SubArray{Int, 1, ...}
@baumgold, this is ready to review if you have a moment. |
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.
This looks good to me, just some small comments. Thank you for this - it's much cleaner and simpler than what I was working on in #405.
(t1 === t2) || (return false) | ||
tt1 = Base.nonmissingtype(t1) | ||
tt2 = Base.nonmissingtype(t2) | ||
if t1 == t2 || (tt1 <: AbstractVector && tt2 <: AbstractVector && eltype(tt1) == eltype(tt2)) |
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.
This looks good to me. Small nit-pick... small simplification without the continue:
t1 != t2 && (tt1 <: AbstractVector && tt2 <: AbstractVector && eltype(tt1) != eltype(tt2)) && return false
No description provided.