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

Reportengine CI #65

Closed
wants to merge 14 commits into from
44 changes: 44 additions & 0 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
name: Reportengine tests

on: [push]

jobs:
test:
runs-on: ubuntu-latest

strategy:
fail-fast: false
matrix:
python-version: ["3.12"]

steps:
- name: Checkout repository
uses: actions/checkout@v2

- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v2
with:
python-version: ${{ matrix.python-version }}

- name: Setup Miniconda
uses: conda-incubator/setup-miniconda@v3
with:
activate-environment: test
use-mamba: true
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need both of these?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi @RoyStegeman, thanks for the comments.

The conda environment is needed because of pandoc. Does that answer your question?

Copy link
Member

Choose a reason for hiding this comment

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

That I understood, but why do you need the python one?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right now the first step is needed to fix the python version.
But I guess we could do it directly in the miniconda env.

I can try to do so if you prefer it

Copy link
Member

Choose a reason for hiding this comment

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

Yes it supports a python-version key, so I guess it makes sense to use that


- name: Update Environment
shell: bash -l {0}
run: |
conda env update -n test -f environment.yml
Copy link
Member

Choose a reason for hiding this comment

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

Oh I thought I wrote a comment on this as well. Anyway, why do you not just do conda install pandoc followed by pip install . instead of declaring the dependencies in an evironment.yml?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi Roy, sorry for the very late reply. I somehow forgot about this PR.

I think that having the environment.yml has the advantage of allowing the user to install reportengine and creating a conda env at the same time:

conda env create -f environment.yml

If we agree on keeping the env file, then why not using it directly in the CI?

Copy link
Member

Choose a reason for hiding this comment

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

Also a long time ago for me...

Anyway, the dependencies are already defined in meta.yaml for installing with conda and pyproject.toml for python. In the test you already build the conda package, so you could create an env using the local package and run the test on that. That way you don't have to define the dependencies in yet another place.

Also conda install --only-deps exists, so people could use that to install deps instead of environment.yaml.

Copy link
Member Author

Choose a reason for hiding this comment

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

Can you be a bit more specific please?
I am not sure the conda package is built in this test. I am only setting up miniconda.


- name: Install pytest dependencies
shell: bash -l {0}
run: |
pip install pytest
pip install hypothesis

- name: Run Tests
shell: bash -l {0}
run: |
pytest

19 changes: 19 additions & 0 deletions environment.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
name: reportengine

channels:
- conda-forge


dependencies:
- python
- flit
- jinja2
- ruamel_yaml =0.15
- matplotlib
- pandas >=1
- pygments
- blessings
- pandoc >=2
- dask
- pip:
- .
Loading