From 658bd7be76a3f8deab37e4999414cfefd836cb9a Mon Sep 17 00:00:00 2001 From: Phillip Cloud <417981+cpcloud@users.noreply.github.com> Date: Fri, 7 Jan 2022 09:30:32 -0500 Subject: [PATCH 1/3] ci: add more variations to ci --- .github/workflows/test.yaml | 95 ++++++++++++++++++++++++++----------- src/context.rs | 3 +- src/expression.rs | 6 +-- src/functions.rs | 6 ++- 4 files changed, 77 insertions(+), 33 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index c1851ec..f9fdab6 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -22,41 +22,42 @@ on: pull_request: branches: [ main ] +concurrency: + group: ${{ github.repository }}-${{ github.head_ref || github.sha }}-${{ github.workflow }} + cancel-in-progress: true + jobs: - test: + test-from-toolchain-file: runs-on: ubuntu-latest - strategy: - fail-fast: false - matrix: - python-version: - - "3.7" - - "3.10" steps: - uses: actions/checkout@v2 - - name: Cache Cargo - uses: actions/cache@v2 + - uses: actions-rs/toolchain@v1 + id: rust-toolchain with: - path: /home/runner/.cargo - key: cargo-maturin-cache-${{ hashFiles('Cargo.lock') }} + override: true + components: rustfmt, clippy - - name: Setup Rust Toolchain - uses: actions-rs/toolchain@v1 + - uses: actions/setup-python@v2 + id: install-python with: - toolchain: stable - profile: default - override: true + python-version: "3.10" - - name: Cargo Clippy - run: cargo clippy + - name: Cache Cargo + uses: actions/cache@v2 + with: + path: ~/.cargo + key: cargo-cache-${{ steps.rust-toolchain.outputs.rustc_hash }}-${{ hashFiles('Cargo.lock') }} - - name: Cargo Check - run: cargo check --all --frozen --locked + - uses: actions-rs/cargo@v1 + with: + command: fmt + args: -- --check - - name: Setup Python Toolchain - uses: actions/setup-python@v2 + - uses: actions-rs/cargo@v1 with: - python-version: ${{ matrix.python-version }} + command: clippy + args: --all-targets --all-features -- -D clippy::all - name: Create Virtualenv run: | @@ -65,7 +66,6 @@ jobs: pip install -r requirements.txt - name: Run Linters - if: ${{ matrix.python-version == '3.10' }} run: | source venv/bin/activate flake8 --exclude venv --ignore=E501 @@ -76,6 +76,47 @@ jobs: source venv/bin/activate maturin develop --cargo-extra-args='--locked' RUST_BACKTRACE=1 pytest -v . - env: - CARGO_HOME: "/home/runner/.cargo" - CARGO_TARGET_DIR: "/home/runner/target" + + test-matrix: + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + python-version: + - "3.7" + - "3.10" + toolchain: + - stable + - beta + - nightly + steps: + - uses: actions/checkout@v2 + + - uses: actions-rs/toolchain@v1 + id: rust-toolchain + with: + toolchain: ${{ matrix.toolchain }} + override: true + + - uses: actions/setup-python@v2 + id: install-python + with: + python-version: ${{ matrix.python-version }} + + - name: Cache Cargo + uses: actions/cache@v2 + with: + path: ~/.cargo + key: cargo-cache-${{ steps.rust-toolchain.outputs.rustc_hash }}-${{ hashFiles('Cargo.lock') }} + + - name: Create Virtualenv + run: | + python -m venv venv + source venv/bin/activate + pip install -r requirements.txt + + - name: Run tests + run: | + source venv/bin/activate + maturin develop --cargo-extra-args='--locked' + RUST_BACKTRACE=1 pytest -v . diff --git a/src/context.rs b/src/context.rs index 98c79d9..ebd893c 100644 --- a/src/context.rs +++ b/src/context.rs @@ -99,6 +99,7 @@ impl PyExecutionContext { Ok(()) } + #[allow(clippy::too_many_arguments)] #[args( schema = "None", has_header = "true", @@ -119,7 +120,7 @@ impl PyExecutionContext { ) -> PyResult<()> { let path = path .to_str() - .ok_or(PyValueError::new_err("Unable to convert path to a string"))?; + .ok_or_else(|| PyValueError::new_err("Unable to convert path to a string"))?; let delimiter = delimiter.as_bytes(); if delimiter.len() != 1 { return Err(PyValueError::new_err( diff --git a/src/expression.rs b/src/expression.rs index 45dea58..0aa936b 100644 --- a/src/expression.rs +++ b/src/expression.rs @@ -37,9 +37,9 @@ impl From for Expr { } } -impl Into for Expr { - fn into(self) -> PyExpr { - PyExpr { expr: self } +impl From for PyExpr { + fn from(expr: Expr) -> PyExpr { + PyExpr { expr } } } diff --git a/src/functions.rs b/src/functions.rs index 7287274..14b1dda 100644 --- a/src/functions.rs +++ b/src/functions.rs @@ -119,11 +119,13 @@ fn window( expr: datafusion::logical_plan::Expr::WindowFunction { fun, args: args.into_iter().map(|x| x.expr).collect::>(), - partition_by: partition_by.unwrap_or_default() + partition_by: partition_by + .unwrap_or_default() .into_iter() .map(|x| x.expr) .collect::>(), - order_by: order_by.unwrap_or_default() + order_by: order_by + .unwrap_or_default() .into_iter() .map(|x| x.expr) .collect::>(), From 21b825d318ff89263bd2c7a3a33ff5c9d60bf9c7 Mon Sep 17 00:00:00 2001 From: Phillip Cloud <417981+cpcloud@users.noreply.github.com> Date: Wed, 12 Jan 2022 21:36:35 -0500 Subject: [PATCH 2/3] Remove toolchain file job --- .github/workflows/test.yaml | 78 ++++++++++++------------------------- 1 file changed, 25 insertions(+), 53 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index f9fdab6..a023e86 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -27,56 +27,6 @@ concurrency: cancel-in-progress: true jobs: - test-from-toolchain-file: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - - - uses: actions-rs/toolchain@v1 - id: rust-toolchain - with: - override: true - components: rustfmt, clippy - - - uses: actions/setup-python@v2 - id: install-python - with: - python-version: "3.10" - - - name: Cache Cargo - uses: actions/cache@v2 - with: - path: ~/.cargo - key: cargo-cache-${{ steps.rust-toolchain.outputs.rustc_hash }}-${{ hashFiles('Cargo.lock') }} - - - uses: actions-rs/cargo@v1 - with: - command: fmt - args: -- --check - - - uses: actions-rs/cargo@v1 - with: - command: clippy - args: --all-targets --all-features -- -D clippy::all - - - name: Create Virtualenv - run: | - python -m venv venv - source venv/bin/activate - pip install -r requirements.txt - - - name: Run Linters - run: | - source venv/bin/activate - flake8 --exclude venv --ignore=E501 - black --line-length 79 --diff --check . - - - name: Run tests - run: | - source venv/bin/activate - maturin develop --cargo-extra-args='--locked' - RUST_BACKTRACE=1 pytest -v . - test-matrix: runs-on: ubuntu-latest strategy: @@ -92,14 +42,15 @@ jobs: steps: - uses: actions/checkout@v2 - - uses: actions-rs/toolchain@v1 + - name: Setup Rust Toolchain + uses: actions-rs/toolchain@v1 id: rust-toolchain with: toolchain: ${{ matrix.toolchain }} override: true - - uses: actions/setup-python@v2 - id: install-python + - name: Setup Python + uses: actions/setup-python@v2 with: python-version: ${{ matrix.python-version }} @@ -109,12 +60,33 @@ jobs: path: ~/.cargo key: cargo-cache-${{ steps.rust-toolchain.outputs.rustc_hash }}-${{ hashFiles('Cargo.lock') }} + - name: Check Formatting + uses: actions-rs/cargo@v1 + if: ${{ python-version == '3.10' && toolchain == 'stable' }} + with: + command: fmt + args: -- --check + + - name: Run Clippy + uses: actions-rs/cargo@v1 + if: ${{ python-version == '3.10' && toolchain == 'stable' }} + with: + command: clippy + args: --all-targets --all-features -- -D clippy::all + - name: Create Virtualenv run: | python -m venv venv source venv/bin/activate pip install -r requirements.txt + - name: Run Python Linters + if: ${{ python-version == '3.10' && toolchain == 'stable' }} + run: | + source venv/bin/activate + flake8 --exclude venv --ignore=E501 + black --line-length 79 --diff --check . + - name: Run tests run: | source venv/bin/activate From 3b990615151394559fa88ca16af7e152ac76ecd6 Mon Sep 17 00:00:00 2001 From: Phillip Cloud <417981+cpcloud@users.noreply.github.com> Date: Wed, 12 Jan 2022 21:37:23 -0500 Subject: [PATCH 3/3] Fix matrix references --- .github/workflows/test.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index a023e86..0c81e26 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -62,14 +62,14 @@ jobs: - name: Check Formatting uses: actions-rs/cargo@v1 - if: ${{ python-version == '3.10' && toolchain == 'stable' }} + if: ${{ matrix.python-version == '3.10' && matrix.toolchain == 'stable' }} with: command: fmt args: -- --check - name: Run Clippy uses: actions-rs/cargo@v1 - if: ${{ python-version == '3.10' && toolchain == 'stable' }} + if: ${{ matrix.python-version == '3.10' && matrix.toolchain == 'stable' }} with: command: clippy args: --all-targets --all-features -- -D clippy::all @@ -81,7 +81,7 @@ jobs: pip install -r requirements.txt - name: Run Python Linters - if: ${{ python-version == '3.10' && toolchain == 'stable' }} + if: ${{ matrix.python-version == '3.10' && matrix.toolchain == 'stable' }} run: | source venv/bin/activate flake8 --exclude venv --ignore=E501