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

Install only the needed lint tool in style checks CI #553

Closed
wants to merge 3 commits into from
Closed

Install only the needed lint tool in style checks CI #553

wants to merge 3 commits into from

Conversation

weiji14
Copy link
Contributor

@weiji14 weiji14 commented Jun 3, 2022

Instead of installing all of torchgeo's required dependencies, just install the lint tool (black/flake8/isort/pydocstyle/pyupgrade) directly.

Should make for a faster style checks CI. From 1m50s to 32s. Compare https://github.com/microsoft/torchgeo/actions/runs/2367868140 vs https://github.com/microsoft/torchgeo/actions/runs/2437296402

As per #552 (comment)

@adamjstewart
Copy link
Collaborator

One downside of this, and I think the reason I didn't do this originally, is that it becomes yet another place we need to list our dependencies. We can't easily manage these dependencies with dependabot, and we won't be able to test minimal version compatibility. Of course, these deps are much less critical for users, so the faster tests may outweigh the centralized dep management.

@weiji14 weiji14 marked this pull request as ready for review June 3, 2022 22:12
@adamjstewart
Copy link
Collaborator

I'm investigating dependabot now. If it lets us have multiple files, e.g.:

  • requirements/requirements.txt (defaults)
  • requirements/requirements-style.txt (style tests)
  • requirements/requirements-tests.txt (pytest, mypy)

we may be able to get the best of both worlds.

@adamjstewart
Copy link
Collaborator

Update: it doesn't let us have multiple files. I wonder if we can grep the specific version requirements we need from a single requirements.txt so that we don't install the latest version of things.

@weiji14
Copy link
Contributor Author

weiji14 commented Jun 6, 2022

Closing as supersed by changes in #551

@weiji14 weiji14 closed this Jun 6, 2022
@weiji14 weiji14 deleted the ci/faster-lint branch June 6, 2022 13:48
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