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

fix python linting using venv #1331

Merged
merged 5 commits into from
Oct 14, 2024

Conversation

mira-miracoli
Copy link
Contributor

@mira-miracoli mira-miracoli commented Oct 10, 2024

Should fix this:

Run pip install --upgrade pip
error: externally-managed-environment

× This environment is externally managed
╰─> To install Python packages system-wide, try apt install
    python3-xyz, where xyz is the package you are trying to
    install.
    
    If you wish to install a non-Debian-packaged Python package,
    create a virtual environment using python3 -m venv path/to/venv.
    Then use path/to/venv/bin/python and path/to/venv/bin/pip. Make
    sure you have python3-full installed.
    
    If you wish to install a non-Debian packaged Python application,
    it may be easiest to use pipx install xyz, which will manage a
    virtual environment for you. Make sure you have pipx installed.
    
    See /usr/share/doc/python3.12/README.venv for more information.

note: If you believe this is a mistake, please contact your Python installation or OS distribution provider. You can override this, at the risk of breaking your Python installation or OS, by passing --break-system-packages.
hint: See PEP 668 for the detailed specification.
Error: Process completed with exit code 1.

@mira-miracoli mira-miracoli marked this pull request as ready for review October 10, 2024 09:32
@@ -8,6 +8,13 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- name: Create venv
Copy link
Member

Choose a reason for hiding this comment

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

Is this the correct way of doing it, or should we use something like this action for it?

https://github.com/galaxyproject/iwc/blob/main/.github/workflows/setup.yml#L63C1-L65C53

@arash77 you know these things correct?

Copy link
Contributor

Choose a reason for hiding this comment

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

Both ways will work to set up a virtual environment, but you're right that using actions/setup-python is better. It handles the setup automatically and makes sure everything is ready without needing to manually activate the virtual environment.

Comment on lines 11 to 12
- name: Setup Python
uses: actions/setup-python@v5
Copy link
Contributor Author

@mira-miracoli mira-miracoli Oct 10, 2024

Choose a reason for hiding this comment

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

Suggested change
- name: Setup Python
uses: actions/setup-python@v5
- name: Setup Python
uses: actions/setup-python@v5
with:
cache: 'pip'
cache-dependency-path: '.github/requirements-python-lint.txt'

Copy link
Contributor

Choose a reason for hiding this comment

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

For chacing pip, it would be better to use requirements.txt instead of direct pip install.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is that stored in the repo's root or under .github? (sorry I tried to google this but did not find anything)

Copy link
Contributor

Choose a reason for hiding this comment

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

It could be anywhere but the address is based on the repo's root. For example, it may be saved in the .github folder, and used like this:
pip install -r .github/requirements.txt

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 think this should do it now. Thanks for showing me :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In general, we could also use the super-linter which I did for WALLE, but I did not want to change too much in the beginning and just fix the action.

@mira-miracoli mira-miracoli merged commit 08a874d into usegalaxy-eu:master Oct 14, 2024
2 checks passed
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.

3 participants