biopython / biopython

Official git repository for Biopython (originally converted from CVS)
http://biopython.org/
Other
4.37k stars 1.75k forks source link

Apply black to Tests/ #2552

Closed peterjc closed 4 years ago

peterjc commented 4 years ago

This is a spin out from #2008, where we have agreed to adopt the black code formatting tool for Biopython:

https://github.com/psf/black

That issue focused on applying black to the main code base (Bio/, BioSQL/, and also Scripts/ and Doc/examples/).

This issue is for applying black to the Biopython test suite.

In principle this is a good issue for new contributors. However, many of the tests include expected data values, so compared to the Biopython main code there are more likely to be cases where black does a bad job - which will need some discussion (e.g. can it be improved with extra brackets, or do we turn off black for a portion of a test file?).

You will need to install black, flake8, and associated plugins as described in https://github.com/biopython/biopython/blob/master/CONTRIBUTING.rst - and we recommend turning on the git pre-commit hook).

Example workflow:

git checkout master # and make sure it is up to date
git checkout -b black_test_XXX # start a new branch
black Tests/test_XXX.py  # do the conversion
flake8 Tests/test_XXX.py # check for string concatenation etc
emacs Tests/test_XXX.py  # review and make any fixes
git commit Tests/test_XXX.py -m "Apply black to Tests/test_XXX.py"
git push my_fork black_text_XXX  # push the branch to your personal GitHub

To avoid duplication of effort, please comment here if you are going to work on a particular file, or group of files.

svalqui commented 4 years ago

I'll work on test_PDB*

svalqui commented 4 years ago

I'll work on test_SeqIO*

svalqui commented 4 years ago

I'll work on test_SearchIO* files.

svalqui commented 4 years ago

I'll work on test_pro* files.

svalqui commented 4 years ago

I'll work on test_Phylo* files.

svalqui commented 4 years ago

I'll work on test_PAML* files.

svalqui commented 4 years ago

I'll work on test_T* files.

svalqui commented 4 years ago

I'll skip test_Tutorial.py as is raising errors with some doctest on the tutorial. ValueError: 4 Tutorial doctests failed: test_chapter_entrez_line_00202, test_chapter_entrez_line_00325, test_chapter_entrez_line_00419, test_chapter_entrez_line_00466,

svalqui commented 4 years ago

I'll work on SCOP files

peterjc commented 4 years ago

You can try python run_tests.py test_Tutoturial.py --offline but it sounds like four of the online Entrez tests in the Tutorial were failing. Please confirm if that was only after running black or not. If not, please open a new issue. Thanks.

svalqui commented 4 years ago

It was before running black, I have logged an issue for that.

svalqui commented 4 years ago

I'll work on test_PopGen* files

svalqui commented 4 years ago

I'll work on test_phenotype* files.

svalqui commented 4 years ago

I'll work on test_NCBI* files.

svalqui commented 4 years ago

test_NCBI_qblast.py is failing, before and after black, so not black related, I'll apply black style and then log an issue.

svalqui commented 4 years ago

I'll work on test_m* files.

svalqui commented 4 years ago

I'll work on test_l* files.

svalqui commented 4 years ago

I'll work on test_k* files.

svalqui commented 4 years ago

I'll work on test_HMM* files.

svalqui commented 4 years ago

I'll work on test_g* files.

svalqui commented 4 years ago

I'll work on test_Entrez files.

svalqui commented 4 years ago

I'll work on test_Emboss files.

svalqui commented 4 years ago

I'll work on test_BioSQL files.

svalqui commented 4 years ago

I'll work on test_AlignIO files.

svalqui commented 4 years ago

All that could had been grouped had black style applied, now looking at individual files.

gandalfthegrape commented 4 years ago

Hello, I'm still new to contributing. Unless I'm mistaken, it looks like test_Nexus.py has not been updated yet. I'm a little confused as to what to do though. When I run black and then "flake8 test_Nexus.py" it only shows 4 lines with trailing whitespace, but when I fix that and go to commit, the pre-commit hook complains about all the missing docstrings for public classes and functions. Glancing at some of the other users work, it looks like some of them also have classes with missing docstrings, eg. class TestSwissProt in test_SwissProt.py.

This file also has the issue of black wanting to expand all the number lists to many lines, but I'll worry about that later.

Any guidance is appreciated!

MarkusPiotrowski commented 4 years ago

Are you using the .flake8 config file of the master branch? E.g. the errors you mentioned are, afaik, turned off for the test folder.

gandalfthegrape commented 4 years ago

I'm not quite sure. When I run flake8 Tests/test_Nexus.py it does not show errors, but when I go to commit it gives me several errors that the .flake8 file should be ignoring.

svalqui commented 4 years ago

The file .flake8 has a section where list some errors that can be ignored: https://github.com/biopython/biopython/blob/6c75d2da18afdc89b6b3ef994d87c601f856ae27/.flake8#L45-L48 for example if you run flake and get this: test_Nexus.py:1168:1: D102 Missing docstring in public method you can ignore it, that relates to missing documentation, a missing docstring in a method not this issue.

if you get this: test_Nexus.py:727:46: W291 trailing whitespace then you should fix it, this is not been ignored, most likely a space in the wrong place that makes the code not PEP compliant or looks odd.

This issue relates to the styling of the code only, black makes the code look even, however in this project the results of black styling is not necessary better, for example some list are presented as one item per line making long lists hard to read, that might need adjustment, core members or contributors will advice accordingly.

I hope this helps.

gandalfthegrape commented 4 years ago

Thanks you for the advice! That makes sense, I couldn't figure out how to force a commit when it gives those errors I mentioned, but I realized I can use the --no-verify argument when I'm sure those are the only errors. I'll get working on this then.

MarkusPiotrowski commented 4 years ago

Probably, your pre-commit hook is set wrongly. Have you tried to remove and re-set it? Are you in the \biopython folder when doing the commit? I mean, the place from where you type git commit ...

peterjc commented 4 years ago

One simple possibility: It could be you had a flake8 error, did the git add, git commit - and got the warning. If you then fixed the flake8 error, and repeated git commit without doing git add first, it would still be trying to commit the bad version, not the newer fixed version. If so, do another git add, then retry the git commit.

peterjc commented 4 years ago

Many thanks to @svalqui - just merged a bunch of their pull requests for this issue which had been left pending.

martonlanga commented 4 years ago

Went through the files, to see which ones need black. 120/210 already done! Feel free to tag me if you make a PR, and I'll edit this to make it up to date.

svalqui commented 4 years ago

I'll work on test_psw.py

svalqui commented 4 years ago

I'll work on test_phyml_tool.py

svalqui commented 4 years ago

I'll work on test_pairwise2_no_C.py

svalqui commented 4 years ago

I'll work on test_codonalign.py

svalqui commented 4 years ago

I'll work on test_cellosaurus.py

svalqui commented 4 years ago

I'll work on test_Ace.py

svalqui commented 4 years ago

I'll work on test_Cluster.py

svalqui commented 4 years ago

I'll work on test_seq.py

jvfe commented 4 years ago

I'm new to contributing but I'll try working on these: test_Prank_tool.py,test_Dialign_tool.py, test_Affy.py, test_NACCESS_tool.py

svalqui commented 4 years ago

I'll work on test_CodonTable.py

peterjc commented 4 years ago

I've added this issue as a suggestion for the BCC2020 CoFest, since it is relatively straightforward to help with.

jvfe commented 4 years ago

I'll work on test_SeqIO_Xdna.py, test_SeqIO_write.py and test_PDB_MMCIFParser.py

chrisbarnettster commented 4 years ago

I'll work on:

https://github.com/biopython/biopython/pull/3109

jvfe commented 4 years ago

Not an extensive list, but from Tests/test_* I believe these are the ones remaining (checked if someone's already working on them):

padix-key commented 4 years ago

I will work on Tests/test_GenBank.py.

peterjc commented 4 years ago

The end is in sight. Some will be tiny changes as the file was previously black formatted but some minor edits were done which didn't quite follow the style (and we missed that because it isn't yet enforced on the Tests/ folder).