Skip to content
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

Move to qgridnext #2814

Merged
merged 9 commits into from
Sep 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 25 additions & 21 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ name: tests
on:
push:
branches:
- '*'
- "*"
pull_request:
branches:
- '*'
- "*"
types:
- opened
- reopened
Expand All @@ -25,18 +25,17 @@ on:
default: false

env:
CACHE_NUMBER: 0 # increase to reset cache manually
PYTEST_FLAGS: --tardis-refdata=${{ github.workspace }}/tardis-refdata --tardis-regression-data=${{ github.workspace }}/tardis-regression-data --cov=tardis --cov-report=xml --cov-report=html --cov-append
CACHE_NUMBER: 0 # increase to reset cache manually
PYTEST_FLAGS: --tardis-refdata=${{ github.workspace }}/tardis-refdata --tardis-regression-data=${{ github.workspace }}/tardis-regression-data --cov=tardis --cov-report=xml --cov-report=html --cov-append
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}

defaults:
run:
shell: bash -l {0}

concurrency:
group: ${{ github.workflow }}-${{ github.head_ref || github.ref }}
cancel-in-progress: true


jobs:
tests:
Expand All @@ -47,11 +46,11 @@ jobs:
fail-fast: false
matrix:
label: [osx-arm64, linux-64]
continuum: ['not', '']
rpacket_tracking: ['not', '']
continuum: ["not", ""]
rpacket_tracking: ["not", ""]
exclude:
- continuum: ''
rpacket_tracking: ''
- continuum: ""
rpacket_tracking: ""
include:
- label: osx-arm64
os: macos-latest
Expand All @@ -65,29 +64,34 @@ jobs:

- name: Setup LFS
uses: ./.github/actions/setup_lfs

- name: Setup environment
uses: ./.github/actions/setup_env
with:
os-label: ${{ matrix.label }}

- name: Install package editable
if: ${{ !inputs.pip_git }}
run: |
pip install -e . --user
- name: Install package git
if: ${{ inputs.pip_git }}
run: |
pip install git+https://github.com/tardis-sn/tardis.git@master
- name: Install qgridnext
if: ${{ !inputs.pip_git }}
run: |
pip install qgridnext
Comment on lines +83 to +87
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be needed after the change I suggested in env file. Try removing it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried removing this but the tests failed because qgridnext is not installed, this code first installs qgridnext for tests to run. If we don't need this the other option would be to enable pip install -e . --user line in the file but that will install all the dependencies again.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

- name: Run tests
run: pytest tardis ${{ env.PYTEST_FLAGS }} -m "${{ matrix.continuum }} continuum and ${{ matrix.rpacket_tracking }} rpacket_tracking"
run: pytest tardis ${{ env.PYTEST_FLAGS }} -m "${{ matrix.continuum }} continuum and ${{ matrix.rpacket_tracking }} rpacket_tracking"

- name: Refdata Generation tests
run: pytest tardis ${{ env.PYTEST_FLAGS }} --generate-reference -m "${{ matrix.continuum }} continuum and ${{ matrix.rpacket_tracking }} rpacket_tracking"
run: pytest tardis ${{ env.PYTEST_FLAGS }} --generate-reference -m "${{ matrix.continuum }} continuum and ${{ matrix.rpacket_tracking }} rpacket_tracking"
if: contains(github.event.pull_request.labels.*.name, 'run-generation-tests') || github.ref == 'refs/heads/master'

- run: mv .coverage .coverage.${{ strategy.job-index }}

- uses: actions/upload-artifact@v4
Expand All @@ -96,7 +100,7 @@ jobs:
path: |
.coverage*
!.coveragerc
combine_coverage_reports:
needs: [tests]
if: github.repository_owner == 'tardis-sn'
Expand All @@ -110,16 +114,16 @@ jobs:

# will download all artifacts(in this case all are coverage reports)
- uses: actions/download-artifact@v4
with:
with:
path: .
merge-multiple: true

- name: Combine coverage reports
run: |
coverage combine
coverage xml
coverage html
- name: Print report
run: coverage report

Expand Down
1 change: 0 additions & 1 deletion tardis/visualization/widgets/line_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
from astropy import units as u
import numpy as np
import pandas as pd
import qgrid
from plotly import graph_objects as go
from plotly.callbacks import BoxSelector
import ipywidgets as ipw
Expand Down
4 changes: 2 additions & 2 deletions tardis/visualization/widgets/util.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"""Utility classes and functions for widgets."""

import logging
import qgrid
import qgridnext
import ipywidgets as ipw
import asyncio

Expand Down Expand Up @@ -95,7 +95,7 @@ def create_table_widget(
)

# Create the table widget using qgrid
return qgrid.show_grid(
return qgridnext.show_grid(
data,
grid_options=grid_options,
column_options=column_options,
Expand Down
5 changes: 2 additions & 3 deletions tardis_env3.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ channels:
- conda-forge

dependencies:

# WARNING: any change to this section must be applied to the conda-forge
# package recipe at https://github.com/conda-forge/tardis-sn-feedstock

Expand Down Expand Up @@ -38,7 +37,8 @@ dependencies:
- matplotlib-base
- ipywidgets
- plotly
- qgrid
- pip:
- qgridnext

# --- Packages not required for conda-forge recipe ---

Expand Down Expand Up @@ -74,4 +74,3 @@ dependencies:

# Other
- git-lfs

Loading