You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "quinnj (via GitHub)" <gi...@apache.org> on 2023/06/13 02:59:25 UTC
[GitHub] [arrow-julia] quinnj opened a new pull request, #466: Ensure that ArrowTypes.default is defined for Vararg tuples
quinnj opened a new pull request, #466:
URL: https://github.com/apache/arrow-julia/pull/466
Fixes #461. This is the other proposal from what I originally suggested. Though `Tuple{Varag}` is never the concrete type of a value in Julia, it comes up when dealing with structs with fields that have Vararg types; not common at all, but it's allowed and happens.
The reflection code here is a little gross, but it seems to be what Base Julia uses in similar queries to see if a tuple has a vararg element. I've commented out the Arrow/runtests test for now until we bump another version and can then uncomment. Unit tests are added for ArrowTypes in the meantime.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-julia] quinnj commented on pull request #466: Ensure that ArrowTypes.default is defined for Vararg tuples
Posted by "quinnj (via GitHub)" <gi...@apache.org>.
quinnj commented on PR #466:
URL: https://github.com/apache/arrow-julia/pull/466#issuecomment-1588486732
As I noted above, `Tuple{Vararg}` actually isn't a concrete type of any kind of value in Julia, so it doesn't really make sense to talk about what it translates to in the arrow format. At runtime, you will always have some kind of concrete `NTuple{N, T}`, based on how many elements there _actually_ are. Obviously, this could be pretty tricky to serialize to arrow, however, if you had, for example, a `Vector{Tuple{Vararg{String}}}`, with different sized tuples. We could potentially try to switch to treating those as lists instead of fixed-size lists, but it's such a weird/oddball type that it doesn't seem worth it unless someone has a real use-case for it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-julia] Moelf commented on pull request #466: Ensure that ArrowTypes.default is defined for Vararg tuples
Posted by "Moelf (via GitHub)" <gi...@apache.org>.
Moelf commented on PR #466:
URL: https://github.com/apache/arrow-julia/pull/466#issuecomment-1588493523
> Tuple{Vararg} actually isn't a concrete type of any kind of value in Julia, so it doesn't really make sense to talk about what it translates to in the arrow format.
I don't see how this follows, surely we can talk about how much information/promise a type makes, and see what's the most precise Arrow type composition we can map to. In this case, if all you see is a `Vararg{T}`, it's making the same amount of guarantee (as far as serialization to/from bytes is concerned) as `Vector{T}` no?
---
>We could potentially try to switch to treating those as lists instead of fixed-size lists
I guess we would error when people have a column of `Vector{Vararg{T}}` of varying length, that's fine, we can throw error and ask user to explicitly convert to `Vector`, somehow, which would be annoying if it's a field in struct.
For (a contrived) example:
```julia
julia> col1 = [v"2.0-rc1.x", v"2.0-rc1.x.y"]
2-element Vector{VersionNumber}:
v"2.0.0-rc1.x"
v"2.0.0-rc1.x.y"
```
Arrow types certainly have enough flexibility to represent these.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-julia] codecov-commenter commented on pull request #466: Ensure that ArrowTypes.default is defined for Vararg tuples
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #466:
URL: https://github.com/apache/arrow-julia/pull/466#issuecomment-1588451222
## [Codecov](https://app.codecov.io/gh/apache/arrow-julia/pull/466?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#466](https://app.codecov.io/gh/apache/arrow-julia/pull/466?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (b56063e) into [main](https://app.codecov.io/gh/apache/arrow-julia/commit/5532270a27ce33ce98e640f28a8f597680786042?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (5532270) will **decrease** coverage by `82.44%`.
> The diff coverage is `100.00%`.
```diff
@@ Coverage Diff @@
## main #466 +/- ##
==========================================
- Coverage 87.53% 5.09% -82.44%
==========================================
Files 26 25 -1
Lines 3272 3199 -73
==========================================
- Hits 2864 163 -2701
- Misses 408 3036 +2628
```
| [Impacted Files](https://app.codecov.io/gh/apache/arrow-julia/pull/466?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [src/ArrowTypes/test/tests.jl](https://app.codecov.io/gh/apache/arrow-julia/pull/466?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL0Fycm93VHlwZXMvdGVzdC90ZXN0cy5qbA==) | `100.00% <ø> (ø)` | |
| [src/ArrowTypes/src/ArrowTypes.jl](https://app.codecov.io/gh/apache/arrow-julia/pull/466?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL0Fycm93VHlwZXMvc3JjL0Fycm93VHlwZXMuamw=) | `87.70% <100.00%> (+0.85%)` | :arrow_up: |
... and [24 files with indirect coverage changes](https://app.codecov.io/gh/apache/arrow-julia/pull/466/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-julia] Moelf commented on pull request #466: Ensure that ArrowTypes.default is defined for Vararg tuples
Posted by "Moelf (via GitHub)" <gi...@apache.org>.
Moelf commented on PR #466:
URL: https://github.com/apache/arrow-julia/pull/466#issuecomment-1588479939
just curious what does this serialize to in terms of Arrow types? I guess logically it's not different from a list since the length is not known (someone can serialize a column with different length of vargs in each row), thus not a fixed size list ni Arrow-land?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-julia] quinnj merged pull request #466: Ensure that ArrowTypes.default is defined for Vararg tuples
Posted by "quinnj (via GitHub)" <gi...@apache.org>.
quinnj merged PR #466:
URL: https://github.com/apache/arrow-julia/pull/466
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org