ci: test against more compilers, setup clippy and fix clippy lints (d…

…atafusion-contrib#9) * ci: add more variations to ci * Remove toolchain file job * Fix matrix references
mmuru · Jan 13, 2022 · eaf94da · eaf94da
1 parent 4a69119
commit eaf94da
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 27 deletions.
diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml
@@ -22,50 +22,66 @@ on:
   pull_request:
     branches: [ main ]
 
+concurrency:
+  group: ${{ github.repository }}-${{ github.head_ref || github.sha }}-${{ github.workflow }}
+  cancel-in-progress: true
+
 jobs:
-  test:
+  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
 
-      - name: Cache Cargo
-        uses: actions/cache@v2
-        with:
-          path: /home/runner/.cargo
-          key: cargo-maturin-cache-${{ hashFiles('Cargo.lock') }}
-
       - name: Setup Rust Toolchain
         uses: actions-rs/toolchain@v1
+        id: rust-toolchain
         with:
-          toolchain: stable
-          profile: default
+          toolchain: ${{ matrix.toolchain }}
           override: true
 
-      - name: Cargo Clippy
-        run: cargo clippy
-
-      - name: Cargo Check
-        run: cargo check --all --frozen --locked
-
-      - name: Setup Python Toolchain
+      - name: Setup Python
         uses: actions/setup-python@v2
         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: Check Formatting
+        uses: actions-rs/cargo@v1
+        if: ${{ matrix.python-version == '3.10' && matrix.toolchain == 'stable' }}
+        with:
+          command: fmt
+          args: -- --check
+
+      - name: Run Clippy
+        uses: actions-rs/cargo@v1
+        if: ${{ matrix.python-version == '3.10' && matrix.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 Linters
-        if: ${{ matrix.python-version == '3.10' }}
+      - name: Run Python Linters
+        if: ${{ matrix.python-version == '3.10' && matrix.toolchain == 'stable' }}
         run: |
           source venv/bin/activate
           flake8 --exclude venv --ignore=E501
@@ -76,6 +92,3 @@ 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"
diff --git 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
@@ -37,9 +37,9 @@ impl From<PyExpr> for Expr {
     }
 }
 
-impl Into<PyExpr> for Expr {
-    fn into(self) -> PyExpr {
-        PyExpr { expr: self }
+impl From<Expr> for PyExpr {
+    fn from(expr: Expr) -> PyExpr {
+        PyExpr { expr }
     }
 }
 

diff --git 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::<Vec<_>>(),
-            partition_by: partition_by.unwrap_or_default()
+            partition_by: partition_by
+                .unwrap_or_default()
                 .into_iter()
                 .map(|x| x.expr)
                 .collect::<Vec<_>>(),
-            order_by: order_by.unwrap_or_default()
+            order_by: order_by
+                .unwrap_or_default()
                 .into_iter()
                 .map(|x| x.expr)
                 .collect::<Vec<_>>(),