Introduce Black and Flake8 by XingerTang · Pull Request #40 · AlphaGenes/AlphaPeel

XingerTang · 2023-07-12T07:54:34Z

Solve #36.

I tried to implement black formatting hooks for AlphaPeel, I will do the else later.

So far, the tools I have used are only pre-commit and black.
pre-commit is a framework for managing and maintaining multi-language pre-commit hooks, and black is one such pre-commit hook that can be used to format your code, whereas flake8 is another hook used to check the style and quality of the code. Since the pre-commit hook is a kind of client-side hook and the client-side hooks are not copied when you clone a repository, one needs to run the following command to install pre-commit locally,

pip install --user pre-commit
pre-commit install

then the hooks will run every time the user made a commit of AlphaPeel.

I will add the above to the documentation once I finished all the hooks.

XingerTang · 2023-07-12T10:09:17Z

I added the flake8 with the latest few commits.

I configure it to ignore some of the errors raised by flake8 to successfully commit this change, however, there is still something we can do to improve. I list the errors raised I think we can improve below:

setup.py:1:1: F401 'setuptools.Extension' imported but unused
setup.py:1:1: F401 'setuptools.find_packages' imported but unused
src/tinypeel/Peeling/Peeling.py:1:1: F401 'concurrent.futures' imported but unused
src/tinypeel/Peeling/Peeling.py:2:1: F401 'numba.int8' imported but unused
src/tinypeel/Peeling/Peeling.py:2:1: F401 'numba.int64' imported but unused
src/tinypeel/Peeling/Peeling.py:2:1: F401 'numba.optional' imported but unused
src/tinypeel/Peeling/Peeling.py:2:1: F401 'numba.boolean' imported but unused
src/tinypeel/Peeling/Peeling.py:3:1: F401 'numba.experimental.jitclass' imported but unused
src/tinypeel/Peeling/Peeling.py:5:1: F401 'collections.OrderedDict' imported but unused
src/tinypeel/Peeling/Peeling.py:7:1: F401 '..tinyhouse.InputOutput' imported but unused
src/tinypeel/Peeling/Peeling.py:8:1: F401 '..tinyhouse.ProbMath' imported but unused
src/tinypeel/Peeling/Peeling.py:9:1: F401 '..tinyhouse.HaplotypeOperations' imported but unused
src/tinypeel/Peeling/Peeling.py:11:1: F401 'math' imported but unused
src/tinypeel/Peeling/Peeling.py:30:5: F841 local variable 'e16' is assigned to but never used
src/tinypeel/Peeling/PeelingInfo.py:1:1: F401 'concurrent.futures' imported but unused
src/tinypeel/Peeling/PeelingInfo.py:2:1: F401 'numba.int8' imported but unused
src/tinypeel/Peeling/PeelingInfo.py:7:1: F401 '..tinyhouse.InputOutput' imported but unused
src/tinypeel/Peeling/PeelingInfo.py:11:1: F401 'math' imported but unused
src/tinypeel/Peeling/PeelingInfo.py:163:5: F841 local variable 'changed' is assigned to but never used
src/tinypeel/Peeling/PeelingUpdates.py:1:1: F401 'concurrent.futures' imported but unused
src/tinypeel/Peeling/PeelingUpdates.py:2:1: F401 'numba.float32' imported but unused
src/tinypeel/Peeling/PeelingUpdates.py:2:1: F401 'numba.int8' imported but unused
src/tinypeel/Peeling/PeelingUpdates.py:2:1: F401 'numba.int64' imported but unused
src/tinypeel/Peeling/PeelingUpdates.py:2:1: F401 'numba.optional' imported but unused
src/tinypeel/Peeling/PeelingUpdates.py:2:1: F401 'numba.boolean' imported but unused
src/tinypeel/Peeling/PeelingUpdates.py:3:1: F401 'numba.experimental.jitclass' imported but unused
src/tinypeel/Peeling/PeelingUpdates.py:5:1: F401 'collections.OrderedDict' imported but unused
src/tinypeel/Peeling/PeelingUpdates.py:9:1: F401 '..tinyhouse.HaplotypeOperations' imported but unused
src/tinypeel/Peeling/PeelingUpdates.py:13:1: F401 'math' imported but unused
src/tinypeel/Peeling/PeelingUpdates.py:255:5: F821 undefined name 'setupTransmission'
src/tinypeel/Peeling/PeelingUpdates.py:272:5: F841 local variable 'patValid' is assigned to but never used
src/tinypeel/tinypeel.py:2:1: F401 'numba.jit' imported but unused
src/tinypeel/tinypeel.py:2:1: F401 'numba.float32' imported but unused
src/tinypeel/tinypeel.py:2:1: F401 'numba.int32' imported but unused
src/tinypeel/tinypeel.py:2:1: F401 'numba.int64' imported but unused
src/tinypeel/tinypeel.py:2:1: F401 'numba.optional' imported but unused
src/tinypeel/tinypeel.py:3:1: F401 'numba.experimental.jitclass' imported but unused
src/tinypeel/tinypeel.py:6:1: F401 '.tinyhouse.ProbMath' imported but unused
src/tinypeel/tinypeel.py:68:17: F841 local variable 'results' is assigned to but never used
src/tinypeel/tinypeel.py:194:5: F841 local variable 'segInfo' is assigned to but never used

XingerTang · 2023-07-12T11:17:21Z

Even though pre-commit forks are done locally once commits are made, there are still other ways to implement it automatically.

tstrait did this via the pre-commit action in its tests workflow. So every time a pull request or a push to its main branch is created, the pre-commit hooks specified in its .pre-commit-config.yaml file would run.

However, the homepage of the pre-commit action says

this action is in maintenance-only mode and will not be accepting new features.

generally you want to use pre-commit.ci which is faster and has more features.

And pre-commit.ci is a GitHub application that you can use with only .pre-commit-config.yaml file to configure everything. By managing your repositories pre-commit.ci settings via clicking the Sign In With GitHub button on its homepage, the pre-commit forks would be triggered automatically every time a push or a pull request is created.

So, in summary, there are two ways to automate the pre-commit forks we used, one is via the pre-commit action, the other one is via pre-commit.ci. Which one do you think is better? @gregorgorjanc

XingerTang · 2023-07-12T13:12:18Z

I just noticed that the devel branch is behind the main branch with a few commits, I rebased it onto the main branch.

gregorgorjanc · 2023-07-13T06:41:56Z

I added the flake8 with the latest few commits.

I configure it to ignore some of the errors raised by flake8 to successfully commit this change, however, there is still something we can do to improve. I list the errors raised I think we can improve below:

setup.py:1:1: F401 'setuptools.Extension' imported but unused
setup.py:1:1: F401 'setuptools.find_packages' imported but unused
src/tinypeel/Peeling/Peeling.py:1:1: F401 'concurrent.futures' imported but unused
src/tinypeel/Peeling/Peeling.py:2:1: F401 'numba.int8' imported but unused
src/tinypeel/Peeling/Peeling.py:2:1: F401 'numba.int64' imported but unused
src/tinypeel/Peeling/Peeling.py:2:1: F401 'numba.optional' imported but unused
src/tinypeel/Peeling/Peeling.py:2:1: F401 'numba.boolean' imported but unused
src/tinypeel/Peeling/Peeling.py:3:1: F401 'numba.experimental.jitclass' imported but unused
src/tinypeel/Peeling/Peeling.py:5:1: F401 'collections.OrderedDict' imported but unused
src/tinypeel/Peeling/Peeling.py:7:1: F401 '..tinyhouse.InputOutput' imported but unused
src/tinypeel/Peeling/Peeling.py:8:1: F401 '..tinyhouse.ProbMath' imported but unused
src/tinypeel/Peeling/Peeling.py:9:1: F401 '..tinyhouse.HaplotypeOperations' imported but unused
src/tinypeel/Peeling/Peeling.py:11:1: F401 'math' imported but unused
src/tinypeel/Peeling/Peeling.py:30:5: F841 local variable 'e16' is assigned to but never used
src/tinypeel/Peeling/PeelingInfo.py:1:1: F401 'concurrent.futures' imported but unused
src/tinypeel/Peeling/PeelingInfo.py:2:1: F401 'numba.int8' imported but unused
src/tinypeel/Peeling/PeelingInfo.py:7:1: F401 '..tinyhouse.InputOutput' imported but unused
src/tinypeel/Peeling/PeelingInfo.py:11:1: F401 'math' imported but unused
src/tinypeel/Peeling/PeelingInfo.py:163:5: F841 local variable 'changed' is assigned to but never used
src/tinypeel/Peeling/PeelingUpdates.py:1:1: F401 'concurrent.futures' imported but unused
src/tinypeel/Peeling/PeelingUpdates.py:2:1: F401 'numba.float32' imported but unused
src/tinypeel/Peeling/PeelingUpdates.py:2:1: F401 'numba.int8' imported but unused
src/tinypeel/Peeling/PeelingUpdates.py:2:1: F401 'numba.int64' imported but unused
src/tinypeel/Peeling/PeelingUpdates.py:2:1: F401 'numba.optional' imported but unused
src/tinypeel/Peeling/PeelingUpdates.py:2:1: F401 'numba.boolean' imported but unused
src/tinypeel/Peeling/PeelingUpdates.py:3:1: F401 'numba.experimental.jitclass' imported but unused
src/tinypeel/Peeling/PeelingUpdates.py:5:1: F401 'collections.OrderedDict' imported but unused
src/tinypeel/Peeling/PeelingUpdates.py:9:1: F401 '..tinyhouse.HaplotypeOperations' imported but unused
src/tinypeel/Peeling/PeelingUpdates.py:13:1: F401 'math' imported but unused
src/tinypeel/Peeling/PeelingUpdates.py:255:5: F821 undefined name 'setupTransmission'
src/tinypeel/Peeling/PeelingUpdates.py:272:5: F841 local variable 'patValid' is assigned to but never used
src/tinypeel/tinypeel.py:2:1: F401 'numba.jit' imported but unused
src/tinypeel/tinypeel.py:2:1: F401 'numba.float32' imported but unused
src/tinypeel/tinypeel.py:2:1: F401 'numba.int32' imported but unused
src/tinypeel/tinypeel.py:2:1: F401 'numba.int64' imported but unused
src/tinypeel/tinypeel.py:2:1: F401 'numba.optional' imported but unused
src/tinypeel/tinypeel.py:3:1: F401 'numba.experimental.jitclass' imported but unused
src/tinypeel/tinypeel.py:6:1: F401 '.tinyhouse.ProbMath' imported but unused
src/tinypeel/tinypeel.py:68:17: F841 local variable 'results' is assigned to but never used
src/tinypeel/tinypeel.py:194:5: F841 local variable 'segInfo' is assigned to but never used

@XingerTang This is great work - exactly why I am proposing these tools;) There is always lots to do;)

gregorgorjanc · 2023-07-13T06:48:09Z

And pre-commit.ci is a GitHub application that you can use with only .pre-commit-config.yaml file to configure everything. By managing your repositories pre-commit.ci settings via clicking the Sign In With GitHub button on its homepage, the pre-commit forks would be triggered automatically every time a push or a pull request is created.

So, in summary, there are two ways to automate the pre-commit forks we used, one is via the pre-commit action, the other one is via pre-commit.ci. Which one do you think is better? @gregorgorjanc

@XingerTang I don't have much experience with these tools and their workflow:(

I really like GitHub actions, I think, suggesting we follow that approach;) If we do that, does a developer need to run pre-commit at all on its side or it gets run automatically?

gregorgorjanc · 2023-07-13T06:53:40Z

@XingerTang are these changes for AlphaPeel repo or for tinyhouse repo?

gregorgorjanc · 2023-07-13T06:59:58Z

I just noticed that the devel branch is behind the main branch with a few commits, I rebased it onto the main branch.

@XingerTang I think you did not git pull recent commits from the upstream. Good to know for future;)

I am happy with most changes, I am just confused why we see changes to tinyhouse in this AlphaPeelrepo - will these go to this repo or totinyhouse` repo where they belong - I know its just code formatting so not critical, but these actions then lead to code divergence and one is confused which version is which;)

XingerTang · 2023-07-13T07:16:58Z

@gregorgorjanc It gets run automatically. With the pre-commit action, it will do exactly the same as the PyPI publication, we can set it to run every time a pull request or a push is created.

XingerTang · 2023-07-13T07:40:11Z

@gregorgorjanc I did the git pull, but my upstream is the devel branch instead of the main, which is now a few commit behind main. I will check the status of the branch before I do any changes next time.

I explained the tinypeel in the first two comments of the repo thing. Please have a look at it :)

gregorgorjanc · 2023-07-14T06:36:38Z

@gregorgorjanc It gets run automatically. With the pre-commit action, it will do exactly the same as the PyPI publication, we can set it to run every time a pull request or a push is created.

@XingerTang I think this would be a useful addition! Please add this option.

XingerTang · 2023-07-14T07:13:50Z

@XingerTang Sure! I will do this later!

XingerTang · 2023-07-14T10:09:33Z

@gregorgorjanc Done! The tests workflow I added runs the pre-commit every time a push or a pull request is created.

gregorgorjanc · 2023-07-14T11:25:41Z

@@ -0,0 +1,19 @@
+name: Test the code


Are we "testing" the code here or "formatting" the code or something else? The word "test" is usually used to ensure that the code gives correct results. Here, to my understanding, we are doing more of a syntax check etc. Right? What would be a better term? Evaluate code?

@gregorgorjanc We are evaluating the format of the code now. The reason I used the "tests" here is becasue I expect someday after we might add real code tests into this workflow, so the "checking the format of the code" plus "checking the outcomes of the code" becomes the complete tests workflow.

However, I can change the name of the workflow for now and rename it later when the real tests really applied.

@XingerTang ok leave the file name but please change the name to "Evaluate code syntax (and correctness in the future)". Thanks!

@gregorgorjanc Done!

gregorgorjanc

OK, code formatting is already happening;)

XingerTang added 5 commits July 11, 2023 17:32

Introduce Black

df82296

Introduce flake8

9b25d0d

Fix E266 from flake8

957fc3a

Add comments to the other errors of flake8

b24c9fd

Update black to latest version

a81d2a2

XingerTang changed the title ~~Introduce Black~~ Introduce Black and Flake8 Jul 12, 2023

XingerTang and others added 11 commits July 12, 2023 13:51

Update contributing guide

05b621d

Delete platform specification of the URL

ec62ffc

Update README

d4d33ed

Update typos in contribute.rst

aa369b8

Further polish contribute.rst

9798812

Remove duplicate user guide link and ignore the environment

37ed622

Modify to recalculate language

080ac2e

Change theme of documentation

1a4c10d

Update contribute.rst

3204622

Introduce Black

d2ae984

Fix merging issue

f6fe5bc

gregorgorjanc mentioned this pull request Jul 13, 2023

Polish code for unused modules and variables AlphaGenes/tinyhouse#5

Open