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

Add isort Workflow #134

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

christoph-blessing
Copy link
Member

@christoph-blessing christoph-blessing commented Feb 24, 2022

This pull request adds a isort workflow and makes various improvements to the
linting workflows.

It also adds a new Dockerfile specifically for linting with black and isort
that uses a plain python image as a base. This speeds up the linting workflows
because they no longer need to download the big sinzlab/pytorch image.

Lastly it also sorts all the imports.

with:
version: "22.1.0"
- name: Check code with black
run: touch .env && docker-compose run black
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand this change with respect to the previous behavior. Could you explain what that is doing?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this because you are setting the black version in the requirements-dev, so it's no longer needed here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this will build the Docker image with the black version specified in dev-requirements.txt and then create a container from that image that then checks the code with black.

Copy link
Member Author

Choose a reason for hiding this comment

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

The benefit is that most of the logic is moved from the workflow to Docker which you can also run locally.

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.

2 participants