From 4814daea4040fb44995a9f969a3e7f259642047a Mon Sep 17 00:00:00 2001 From: Phillip Alday Date: Thu, 25 Jan 2024 15:53:03 -0600 Subject: [PATCH 1/7] fix for Arrow 2.7 --- src/samples.jl | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/samples.jl b/src/samples.jl index f5fe8027..5c8b9891 100644 --- a/src/samples.jl +++ b/src/samples.jl @@ -576,7 +576,13 @@ function Arrow.ArrowTypes.JuliaType(::Val{SAMPLES_ARROW_NAME}, ::Type{<:SamplesA end function Arrow.ArrowTypes.fromarrow(::Type{<:Samples}, arrow_data, arrow_info, arrow_encoded) - info = Arrow.ArrowTypes.fromarrow(SamplesInfoV2, arrow_info...) + info = Arrow.ArrowTypes.fromarrow(SamplesInfoV2, arrow_info) data = reshape(arrow_data, (channel_count(info), :)) return Samples(data, info, arrow_encoded) end + +# XXX in Arrow 2.7, the extra indirection with `fromarrowstruct` leads to the named tuple +# being splatted as positional arguments and not keyword args unless we add this method. +# The splatting leads to a method error because SamplesInfoV2 doesn't have a positional +# contructor, just a constructor from a row or from kwargs +Arrow.ArrowTypes.fromarrow(::Type{SamplesInfoV2}, x::NamedTuple) = SamplesInfoV2(x) From c698031336ab0d8a03d431b2078daddd5bb48cda Mon Sep 17 00:00:00 2001 From: Phillip Alday Date: Thu, 25 Jan 2024 15:57:23 -0600 Subject: [PATCH 2/7] add CI --- .github/workflows/CI.yml | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index ee45ffa9..6f57292f 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -9,7 +9,7 @@ on: pull_request: jobs: test: - name: Julia ${{ matrix.version }} - ${{ matrix.os }} - ${{ matrix.arch }} - ${{ github.event_name }} + name: Julia ${{ matrix.version }} - ${{ matrix.os }} - ${{ matrix.arch }} - Arrow - ${{matrix.arrow}} runs-on: ${{ matrix.os }} strategy: fail-fast: false @@ -21,6 +21,9 @@ jobs: - ubuntu-latest arch: - x64 + arrow: + - '2.6' + - 'current' steps: - uses: actions/checkout@v2 with: @@ -34,10 +37,17 @@ jobs: path: ~/.julia/artifacts key: ${{ runner.os }}-test-artifacts-${{ hashFiles('**/Project.toml') }} restore-keys: ${{ runner.os }}-test-artifacts + - name: "install Arrow 2.6" + if: ${{ matrix.version }} == '2.6' + shell: julia --project + run: | + using Pkg + Pkg.add(PackageSpec(; name="Arrow", uuid="69666777-d1a9-59fb-9406-91d4454c9d45", version="2.6")) + Pkg.pin("Arrow") - uses: julia-actions/julia-buildpkg@v1 - uses: julia-actions/julia-runtest@v1 - uses: julia-actions/julia-processcoverage@v1 - - uses: codecov/codecov-action@v2 + - uses: codecov/codecov-action@v3 with: file: lcov.info docs: From a187f848024feb0f985e12a717f8d9a3973ac106 Mon Sep 17 00:00:00 2001 From: Phillip Alday Date: Thu, 25 Jan 2024 15:59:12 -0600 Subject: [PATCH 3/7] more tweaks --- .github/workflows/CI.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 6f57292f..dbbd33d4 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -1,4 +1,7 @@ name: CI +concurrency: + group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }} + cancel-in-progress: true on: push: branches: @@ -39,7 +42,7 @@ jobs: restore-keys: ${{ runner.os }}-test-artifacts - name: "install Arrow 2.6" if: ${{ matrix.version }} == '2.6' - shell: julia --project + shell: julia --project '{0}' run: | using Pkg Pkg.add(PackageSpec(; name="Arrow", uuid="69666777-d1a9-59fb-9406-91d4454c9d45", version="2.6")) From 4f8e70794efcfe4e3aac9d764d410b432dbc86fb Mon Sep 17 00:00:00 2001 From: Phillip Alday Date: Thu, 25 Jan 2024 16:01:06 -0600 Subject: [PATCH 4/7] fix? --- .github/workflows/CI.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index dbbd33d4..ed1682ad 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -42,9 +42,10 @@ jobs: restore-keys: ${{ runner.os }}-test-artifacts - name: "install Arrow 2.6" if: ${{ matrix.version }} == '2.6' - shell: julia --project '{0}' + shell: julia --color=yes '{0}' run: | using Pkg + Pkg.activate(".") Pkg.add(PackageSpec(; name="Arrow", uuid="69666777-d1a9-59fb-9406-91d4454c9d45", version="2.6")) Pkg.pin("Arrow") - uses: julia-actions/julia-buildpkg@v1 From e8c79d2b10533cce176cdeb491f34a5d59b42e67 Mon Sep 17 00:00:00 2001 From: Phillip Alday Date: Thu, 25 Jan 2024 16:02:46 -0600 Subject: [PATCH 5/7] try again --- .github/workflows/CI.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index ed1682ad..0b506884 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -42,7 +42,7 @@ jobs: restore-keys: ${{ runner.os }}-test-artifacts - name: "install Arrow 2.6" if: ${{ matrix.version }} == '2.6' - shell: julia --color=yes '{0}' + shell: julia --color=yes --project {0} run: | using Pkg Pkg.activate(".") From 65179be2435e1b5d98ac22c060c66ab4a5a08664 Mon Sep 17 00:00:00 2001 From: Phillip Alday Date: Thu, 25 Jan 2024 16:08:13 -0600 Subject: [PATCH 6/7] better conditional --- .github/workflows/CI.yml | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 0b506884..5628e14a 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -40,13 +40,12 @@ jobs: path: ~/.julia/artifacts key: ${{ runner.os }}-test-artifacts-${{ hashFiles('**/Project.toml') }} restore-keys: ${{ runner.os }}-test-artifacts - - name: "install Arrow 2.6" - if: ${{ matrix.version }} == '2.6' + - name: "install specific Arrow version" + if: ${{ matrix.arrow != 'current' }} shell: julia --color=yes --project {0} run: | using Pkg - Pkg.activate(".") - Pkg.add(PackageSpec(; name="Arrow", uuid="69666777-d1a9-59fb-9406-91d4454c9d45", version="2.6")) + Pkg.add(PackageSpec(; name="Arrow", uuid="69666777-d1a9-59fb-9406-91d4454c9d45", version="${{ matrix.arrow }}")) Pkg.pin("Arrow") - uses: julia-actions/julia-buildpkg@v1 - uses: julia-actions/julia-runtest@v1 From 8c64ee6e5fd8de959e1d137abbf1fd9afc95969b Mon Sep 17 00:00:00 2001 From: Phillip Alday Date: Fri, 26 Jan 2024 12:04:55 -0600 Subject: [PATCH 7/7] better description of method Co-authored-by: Eric Hanson <5846501+ericphanson@users.noreply.github.com> --- src/samples.jl | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/samples.jl b/src/samples.jl index 5c8b9891..05ba5781 100644 --- a/src/samples.jl +++ b/src/samples.jl @@ -581,8 +581,10 @@ function Arrow.ArrowTypes.fromarrow(::Type{<:Samples}, arrow_data, arrow_info, a return Samples(data, info, arrow_encoded) end -# XXX in Arrow 2.7, the extra indirection with `fromarrowstruct` leads to the named tuple -# being splatted as positional arguments and not keyword args unless we add this method. -# The splatting leads to a method error because SamplesInfoV2 doesn't have a positional -# contructor, just a constructor from a row or from kwargs +# Legolas v0.5.17 removed the `fromarrow` methods for Legolas rows, preferring the new `fromarrowstruct` +# introduced in Arrow v2.7. We don't want to assume Arrow v2.7 is loaded here, so we will add a method +# so that `fromarrow` continues to work for `SamplesInfoV2`. Additionally, this method is agnostic +# to serialization order of fields (which is the benefit `fromarrowstruct` is designed to bring), so +# we retain correctness. Lastly, as this method's signature is different from the one Legolas pre-v0.5.17 +# generates, we avoid method overwriting errors/warnings. Arrow.ArrowTypes.fromarrow(::Type{SamplesInfoV2}, x::NamedTuple) = SamplesInfoV2(x)