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

13 clarifying environment setup in readme #16

Merged
merged 48 commits into from
Feb 7, 2025

Conversation

LevanBokeria
Copy link
Collaborator

The following changes are implemented:

  • pyproject.toml instead of setup.py
  • Updating python requirement to be >=3.10
  • Updating the GitHub CI to run conda as well as venv environment setup prior to running tests.
  • Updating the readme to have steps for environment setup.

@LevanBokeria LevanBokeria linked an issue Jan 24, 2025 that may be closed by this pull request
adding conda init in CI before conda activate
@LevanBokeria
Copy link
Collaborator Author

I'm looking into why the existing CIs are failing.

@LevanBokeria
Copy link
Collaborator Author

@crangelsmith after lots of attempts, I've finally gottent the conda CI pipeline to work as well, but had to use a strange workaround due to some error in getting conda to activate the environment.

Original code I had was the following (its also commented in the ci.yml file if you want to try):

  build-conda:
    runs-on: ubuntu-latest

    steps:
    - name: Checkout code
      uses: actions/checkout@v3

    - name: Set up Conda
      uses: conda-incubator/setup-miniconda@v2
      with:
        python-version: '3.10'

    - name: Create Conda environment
      run: |
        conda create --name myenv python=3.10 -y
    - name: Install dependencies
      run: |
        conda init
        conda activate myenv
        python -m pip install --upgrade pip
        pip install ./
    - name: Run tests
      run: |
        python -m unittest discover -s tests

But this failed with the error: CondaError: Run 'conda init' before 'conda activate'

I tried lots of fixes for this, including reactivation of the shell, different ways of calling the conda activate function, etc. But nothing worked.

Instead, I found a workaround where the build-conda module creates a conda environment with an environment.yml file and activates it within its with statement. Then, under that same module I add a run submodule which does the pip install. Even then, there was an error that required adding a line to upgrade setuptools, but now it all works:

  build-conda:
    name: Conda environment
    runs-on: "ubuntu-latest"
    defaults:
      run:
        shell: bash -el {0}
    steps:
      - uses: actions/checkout@v4
      - uses: conda-incubator/setup-miniconda@v3
        with:
          activate-environment: myenv
          environment-file: environment.yml
          python-version: '3.10'
          auto-activate-base: false
      - run: | 
          python -m pip install --upgrade pip
          pip install setuptools --upgrade
          pip install ./

      - name: Run tests
        run: |    
          python -m unittest discover -s tests

Its not ideal because this workaround requires existence of the environment.yml file, but we want to use the toml file for all the dependencies. So right now, the environment.yml file just specifies the name and thats all.

@LevanBokeria LevanBokeria marked this pull request as draft January 30, 2025 12:18
@LevanBokeria LevanBokeria self-assigned this Feb 4, 2025
@LevanBokeria LevanBokeria marked this pull request as ready for review February 4, 2025 10:18
@LevanBokeria
Copy link
Collaborator Author

Hi team. The PR is ready to be reviewed:

  • Both conda and python virtual environments are being created during the CI.
  • For conda, the environment.yml file gets created on the fly, so we don't track an unnecessary .yml file in the repo.

Max added a new function to the dev branch, so updaing this feature branch to include that.
@MaxBalmus MaxBalmus merged commit 28002af into dev Feb 7, 2025
2 checks passed
@MaxBalmus MaxBalmus deleted the 13-clarifying-environment-setup-in-readme branch February 7, 2025 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clarifying environment setup in README
2 participants