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.

Sign up for GitHub

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

Jump to bottom

Replace assertTrue with assertIn/assertNotIn when suitable #1020

Merged
merged 1 commit into from
Dec 9, 2016
Merged

Replace assertTrue with assertIn/assertNotIn when suitable #1020

merged 1 commit into from
Dec 9, 2016

Conversation

milindl
Copy link
Contributor

@milindl milindl commented Dec 9, 2016 •
edited
Loading

This should close #1006

assertTrue(a in b) is now assertIn(a, b)
assertTrue(a in b, b) is assertIn(a, b)
assertTrue(a in b, message) is assertIn(a, b, message)
The same for 'not in', by using assertNotIn()

In cases where there was any doubt, I just left them unchanged for safety.

Thanks!

peterjc
peterjc reviewed Dec 9, 2016
View reviewed changes
Tests/test_Mafft_tool.py
@@ -87,7 +87,7 @@ def test_Mafft_simple(self):
# became "Progressive alignment 1/2..." and "Progressive alignment 2/2..."
self.assertTrue(("Progressive alignment ..." in stderrdata) or
("Progressive alignment 1/" in stderrdata), stderrdata)
self.assertTrue("$#=0" not in stderrdata)
self.assertNotIn("$#=0" , stderrdata)
Copy link
Member

Choose a reason for hiding this comment

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

You have an extra space before the comma, which is why TravisCI's pep8 check is failing (here and elsewhere)

peterjc
peterjc reviewed Dec 9, 2016
View reviewed changes
Tests/test_Entrez_online.py
recs = list(Entrez.parse(handle))
handle.close()
self.assertEqual(3, len(recs))
# arbitrary number, just to make sure the parser works
self.assertTrue(all(len(rec).keys > 5) for rec in recs)
self.assertIn(all(len(rec).keys > 5) for rec, recs)
Copy link
Member

Choose a reason for hiding this comment

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

This is now giving a syntax error and causing TravisCI to fail the test. This is actually a generator expression inside an all(...) call.

@peterjc
Copy link
Member

peterjc commented Dec 9, 2016

Almost perfect - you need to fix the syntax error in Tests/test_Entrez_online.py line 70, and remove the extra spaces for PEP8 conformance.

To run the PEP8 check locally, pip install pep8 and then this will match what we're currently enforcing on TravisCI via Tox:

$ pep8 --ignore E122,E123,E126,E127,E128,E241,E402,E501,E731 Tests/

@peterjc
Copy link
Member

peterjc commented Dec 9, 2016

Once those minor issues are fixed, this should be ready to merge.

Are you happy to dual licence your contributions under both the Biopython License Agreement and the BSD 3-Clause License? See #898.

Also, are you happy to be thanked by name in the NEWS and CONTRIB files?

Thanks!

@milindl
Replace assertTrue with assertIn/assertNotIn when suitable
f325362 
This closes issue 1006
assertTrue(a in b) is now assertIn(a, b)
assertTrue(a in b, b) is assertIn(a, b)
assertTrue(a in b, message) is assertIn(a, b, message)
The same for 'not in', by using assertNotIn
@milindl
Copy link
Contributor Author

milindl commented Dec 9, 2016

I've fixed the spacing and the line 70 problem :)

Yes, I'm happy about both those things.

Thanks!

@peterjc peterjc merged commit 2948de1 into biopython:master Dec 9, 2016
peterjc added a commit that referenced this pull request Dec 9, 2016
@peterjc
Thank Milind for test updates.
fa6d2e7 
See pull request #1020 which closes #1006.
@peterjc peterjc mentioned this pull request Dec 9, 2016
Open
@peterjc
Copy link
Member

peterjc commented Dec 9, 2016

Thank you very very much.

MarkusPiotrowski pushed a commit to MarkusPiotrowski/biopython that referenced this pull request Oct 31, 2017
@peterjc @MarkusPiotrowski
Thank Milind for test updates.
540d367 
See pull request biopython#1020 which closes biopython#1006.
peterjc added a commit to peterjc/biopython that referenced this pull request Aug 30, 2019
@peterjc
Dual license Tests/test_PDB.py
49216ce 
Original authors Thomas Hamelryck and Eric Talevich (for the unittest
conversion), and the contributors as tracked with version control have
agreed:

 - Andrew Guy (@andrewguy)
   biopython#2212 (comment)
 - Ben Reynwar (@benreynwar)
   biopython#898 (comment)
 - Bernhard C. Thiel (@Bernhard10)
   biopython#968
 - Carlos Pena (@carlosp420)
   biopython#898 (comment)
 - Chris Rands (@chris-rands)
   biopython#1301
 - Christian Brueffer (@cbrueffer)
   biopython#898 (comment)
 - Eric Talevich (@etal)
   biopython#898 (comment)
 - Francesco Gastaldello (@Gasta88)
   biopython#1063
 - Gokcen Eraslan (@gokceneraslan)
   biopython#898 (comment)
 - Jack Twilley (@mathuin)
   biopython#1049 (comment)
 - Jeroen Van Goey (@BioGeek)
   biopython#898 (comment)
 - Joe Greener (@jgreener64)
   biopython#1393
 - Joerg Schaarschmidt (@schaarj)
   biopython#1567
 - João Rodrigues (@JoaoRodrigues)
   biopython#898 (comment)
 - Kian Ho (@kianho)
   biopython#898 (comment)
 - Kristian Davidsen (@krdav)
   biopython#898 (comment)
 - Kristian Rother (@krother)
   biopython#898 (comment)
 - Lenna Peterson (@lennax)
   biopython#898 (comment)
 - Markus Piotrowski (@MarkusPiotrowski)
   biopython#898 (comment)
 - Milind Luthra (@milindl)
   biopython#1020
 - Nick Negretti (@nimne)
   biopython#1768 (comment)
 - Peter Cock (@peterjc)
   biopython#898 (comment)
 - Rasmus Fonseca (@RasmusFonseca)
   biopython#1171 (comment)
 - Siong Kong (@siongkong)
   biopython#1516

 - Stefans Mezulis (@StefansM)
   biopython#1445
 - Thomas Hamelryck (CVS username nisse)
   https://mailman.open-bio.org/pipermail/biopython/2018-September/016546.html
peterjc added a commit to peterjc/biopython that referenced this pull request Sep 2, 2019
@peterjc
Dual license Tests/test_Nexus.py
b34354d 
Named copyright holders and other contributors tracked with version
control have all agreed:

 - Benjamin Rowell (@benjaminr)
   biopython#2023
 - Brad Chapman (@chapmanb)
   http://mailman.open-bio.org/pipermail/biopython-dev/2013-July/019903.html
 - Chris Warth (@cswarth)
   biopython#898 (comment)
 - Christian Brueffer (@cbrueffer)
   biopython#898 (comment)
 - Eric Talevich (@etal)
   biopython#898 (comment)
 - Francesco Gastaldello (@Gasta88)
   biopython#1063
 - Frank Kauff (@frankkl)
   biopython#2246 (comment)
 - Iddo Friedberg (@idoerg)
   biopython#898 (comment)
 - Joe Cora (@JoeCora)
   biopython#898 (comment)
 - Michiel de Hoon (@mdehoon)
   biopython#898 (comment)
 - Milind Luthra (@milindl)
   biopython#1020
 - Nick Negretti (@nimne)
   biopython#1768 (comment)
 - Peter Cock (@peterjc)
   biopython#898 (comment)
 - Sourav Singh (@souravsingh)
   biopython#957
 - Steve Bond (@biologyguy)
   biopython#898 (comment)
 - Tiago Antao (@tiagoantao)
   biopython#898 (comment)
 - Vincent Davis (@vincentdavis)
   biopython#898 (comment)
peterjc added a commit that referenced this pull request Nov 4, 2019
@peterjc
Dual license Tests/test_Nexus.py
4a527a0 
Named copyright holders and other contributors tracked with version
control have all agreed:

 - Benjamin Rowell (@benjaminr)
   #2023
 - Brad Chapman (@chapmanb)
   http://mailman.open-bio.org/pipermail/biopython-dev/2013-July/019903.html
 - Chris Warth (@cswarth)
   #898 (comment)
 - Christian Brueffer (@cbrueffer)
   #898 (comment)
 - Eric Talevich (@etal)
   #898 (comment)
 - Francesco Gastaldello (@Gasta88)
   #1063
 - Frank Kauff (@frankkl)
   #2246 (comment)
 - Iddo Friedberg (@idoerg)
   #898 (comment)
 - Joe Cora (@JoeCora)
   #898 (comment)
 - Michiel de Hoon (@mdehoon)
   #898 (comment)
 - Milind Luthra (@milindl)
   #1020
 - Nick Negretti (@nimne)
   #1768 (comment)
 - Peter Cock (@peterjc)
   #898 (comment)
 - Sourav Singh (@souravsingh)
   #957
 - Steve Bond (@biologyguy)
   #898 (comment)
 - Tiago Antao (@tiagoantao)
   #898 (comment)
 - Vincent Davis (@vincentdavis)
   #898 (comment)
peterjc added a commit that referenced this pull request Nov 4, 2019
@peterjc
Dual license Tests/test_PDB.py
55280ed 
Original authors Thomas Hamelryck and Eric Talevich (for the unittest
conversion), and the contributors as tracked with version control have
agreed:

 - Andrew Guy (@andrewguy)
   #2212 (comment)
 - Ben Reynwar (@benreynwar)
   #898 (comment)
 - Bernhard C. Thiel (@Bernhard10)
   #968
 - Carlos Pena (@carlosp420)
   #898 (comment)
 - Chris Rands (@chris-rands)
   #1301
 - Christian Brueffer (@cbrueffer)
   #898 (comment)
 - Eric Talevich (@etal)
   #898 (comment)
 - Francesco Gastaldello (@Gasta88)
   #1063
 - Gokcen Eraslan (@gokceneraslan)
   #898 (comment)
 - Jack Twilley (@mathuin)
   #1049 (comment)
 - Jeroen Van Goey (@BioGeek)
   #898 (comment)
 - Joe Greener (@jgreener64)
   #1393
 - Joerg Schaarschmidt (@schaarj)
   #1567
 - João Rodrigues (@JoaoRodrigues)
   #898 (comment)
 - Kian Ho (@kianho)
   #898 (comment)
 - Kristian Davidsen (@krdav)
   #898 (comment)
 - Kristian Rother (@krother)
   #898 (comment)
 - Lenna Peterson (@lennax)
   #898 (comment)
 - Markus Piotrowski (@MarkusPiotrowski)
   #898 (comment)
 - Milind Luthra (@milindl)
   #1020
 - Nick Negretti (@nimne)
   #1768 (comment)
 - Peter Cock (@peterjc)
   #898 (comment)
 - Rasmus Fonseca (@RasmusFonseca)
   #1171 (comment)
 - Siong Kong (@siongkong)
   #1516

 - Stefans Mezulis (@StefansM)
   #1445
 - Thomas Hamelryck (CVS username nisse)
   https://mailman.open-bio.org/pipermail/biopython/2018-September/016546.html
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peterjc peterjc peterjc left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use self.assertIn in our unit tests
2 participants
@milindl @peterjc