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

Make code flake8, mypy, black and isort compliant 🥸 #12

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

laulopezreal
Copy link
Contributor

pre-commit is a useful tool for managing linters and checking your code for simple errors before comitting. Some advantages are:

Users don't need to install linting tools (other than pre-commit), and tool versions are shared across users / workflows.
pre-commit can be installed locally to prevent committing unless the checks have passed, enforcing a high code quality
Some of the useful built-in hooks are: trim whitespacemypy, pyroma, check-manifest)

@slobentanzer
Copy link
Collaborator

very cool @laulopezreal, many thanks!

we don't have pre-commit in the adapters/pipelines so far, so this is a very welcome addition. will review as soon as possible. :)

Copy link
Collaborator

@kpto kpto left a comment

Choose a reason for hiding this comment

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

@laulopezreal Thanks for your contribution! There are a few issues needed to be sorted.

  1. I think you modified the pyproject.toml directly, this makes the lock file unsync one will not be able to run poetry install. You should run poetry update {package_name} to update a package instead.
  2. The data folder should be omitted by the formatter/linter as they are resources, not code.
  3. The add of file import.report seems to be a mistake to me? I think that should be ignored by Git?

These are my initial comments, I will have a more thorough review later.

@slobentanzer
Copy link
Collaborator

Hi @kpto,
modifying the pyproject.toml directly is definitely possible, I don't think that is the reason for the failure. You can usually just run poetry lock to update.

@slobentanzer
Copy link
Collaborator

The dependency issue I get is that the packages we install from our other git repositories still depend on a previous biocypher version (they are defined as dmb and bccb for the DepMap and CROssBAR repositories). I used these as imports for (originally) demonstrating the workflow of importing adapters from other places on GitHub.

I would suggest to deactivate these imports in the pyproject.toml. I have added these changes to the other PR (the first one, #11).

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