Add 'testseq' function #1306

Adil-Iqbal · 2017-07-03T17:03:26Z

Reverted Seeding.
Updated doctests for reverted seeding.
Reviewed unit test to ensure that it works with reverted seeding.
Allowed table argument to accept names and objects.
Updated unit test to attempt importing normally before using the "manually_import" function.

Everything in this pull request has been discussed earlier with the exception of the very last item. I have wrapped the 'manually_import' function in a try/except block. My reason for doing this is below:

Python has a built-in import utility module that gives access to import internals. Over the last few versions of python, this built-in module has gone through several re-works. My concern is that it will be changed further in the future. If that happens, the 'unit test will cause an import error.

Since importing works fine in Python 3.3 and beyond, I have wrapped the call to the "manually_import" function in a try/except block. The unit test will now attempt to import the module normally before it tries to use the "manually_import" function.

This change will allow the unit test to continue working regardless of whether the import utility module is re-worked in subsequent versions of python.

…ated docstrings.

peterjc · 2017-07-03T17:09:25Z

Tests/run_tests.py

+# Skip doctest for all modules outside of 'Bio' package prior to Python 3.3
+if float(sys.version[:3]) < 3.3:
+    for i, name in enumerate(DOCTEST_MODULES):
+        if is_outside_bio(name):


I don't think the extra function is needed, how about if not name.split(".", 1)[0].startswith("Bio"): here?

Sounds good. Will implement.

Hang on, on reflection this would be clearer: if not name.startswith(("Bio.", "BioSQL.")):

Note this is passing a tuple of two elements as the argument to the starts-with method.

Fewer operations in fewer keystrokes. Awesome!
I will wait until the CI tests are finished before I commit anything further.

peterjc · 2017-07-03T17:10:04Z

Tests/run_tests.py

@@ -70,6 +70,13 @@ def is_numpy():
        return False


+def is_outside_bio(name):
+    name = name.split(".")
+    if "Bio" in name[0]:


I'd use name[0].startswith("Bio") or maybe even name[0] in ["Bio", "BioSQL"] to be explicit?

See later comment on where this function was being used.

2. Remove reference to 'ModuleNotFoundError' 3. Simplify 'manually_import' function for only python 2.7

codecov · 2017-07-03T18:15:10Z

Codecov Report

Merging #1306 into master will increase coverage by 0.88%.
The diff coverage is 88.26%.

@@            Coverage Diff             @@
##           master    #1306      +/-   ##
==========================================
+ Coverage   83.36%   84.24%   +0.88%     
==========================================
  Files         317      319       +2     
  Lines       48642    48912     +270     
==========================================
+ Hits        40551    41208     +657     
+ Misses       8091     7704     -387

Impacted Files	Coverage Δ
Scripts/testseq.py	`88.26% <88.26%> (ø)`
Bio/Wise/__init__.py	`78.94% <0%> (-1.76%)`	⬇️
Bio/NeuralNetwork/Gene/Schema.py	`71.07% <0%> (-0.83%)`	⬇️
Bio/phenotype/pm_fitting.py	`82.45% <0%> (ø)`
Bio/Phylo/BaseTree.py	`89.39% <0%> (+0.19%)`	⬆️
Bio/SeqIO/UniprotIO.py	`92.21% <0%> (+0.29%)`	⬆️
Bio/phenotype/phen_micro.py	`85.22% <0%> (+2.22%)`	⬆️
Bio/Phylo/_utils.py	`67.08% <0%> (+36.7%)`	⬆️
Bio/codonalign/codonseq.py	`87.75% <0%> (+43.74%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 381dae1...9f38c56. Read the comment docs.

Adil-Iqbal · 2017-07-03T19:32:42Z

Alright! Done. Unless anyone has other concerns?

I ended up removing a lot of the code from the "manually_import" function since it just isn't necessary after python version 3.3.

peterjc · 2017-07-03T21:43:57Z

Tests/run_tests.py

@@ -165,6 +166,12 @@ def is_numpy():
    DOCTEST_MODULES.remove("Bio.SeqIO")
    DOCTEST_MODULES.remove("Bio.SearchIO")

+# Skip doctests for all modules outside Bio & BioSQL prior to Python 3.3
+if float(sys.version[:3]) < 3.3:


Sorry, I should have spotted this earlier - you should not assume you can break up the version string like that.

Instead use sys.version_info which is a (named) tuple, i.e. something like this should work?

if sys.version_info < (3, 3):

Not a problem.

peterjc · 2017-07-04T15:18:55Z

The style checks failed on Travis, but it was not your fault:

$ flake8 Bio/
Bio/Phylo/TreeConstruction.py:485:25: W503 line break before binary operator

Its been fixed in c759829

If you are comfortable with the git rebase command, that would be the ideal way to update your branch. However, as the previous commit passed all our tests, I am not worried about it.

Because of the namespace and import issues, I'm going to ask for a second opinion before merging this... http://mailman.open-bio.org/pipermail/biopython-dev/2017-July/021808.html

picrin · 2017-07-06T23:07:22Z

Tests/test_testseq.py

+        module_path = os.path.join(module_path, step)
+    name = name[-1][:-3]
+    import imp
+    return imp.load_source(name, module_path)


Although the method is hacky, I can see it's utility. As more code is added to the "Scripts" directory will we need to call this method from other tests in the "Tests" directory? Would it be more suitable to move this method to a generic "utils" module?

I just see that this can become a common issue with code added to Scripts directory.

If we do end up with more code in Scripts/ being tested like this, then that seems like a good idea. Right now having the code here seems fine to me.

peterjc · 2017-07-07T10:38:50Z

Adam Kurkiewicz on the mailing list http://mailman.open-bio.org/pipermail/biopython-dev/2017-July/021813.html reports a stochastic failure:

I can get the test cases to pass OK 90% of the time inside Tests by
running

python test_testseq.py

However, these test cases are still non-deterministic, in particular
test_codon_tables fails about 10% of the time. Is this intended
behaviour?

That's the stack trace:

picrin@lamport:~/programming/biopython/adil/Tests$ python
test_testseq.py
test_alphabet (__main__.TestTestseq)
Testing 'alphabet' argument... ... ok
test_codon_tables (__main__.TestTestseq)
Testing codon sets... ... FAIL
test_gc_target (__main__.TestTestseq)
Testing 'gc_target' argument... ... ok
test_messenger (__main__.TestTestseq)
Testing 'messenger' argument... ... ok
test_seeding (__main__.TestTestseq)
Testing 'rand_seed' argument... ... ok
test_size (__main__.TestTestseq)
Testing 'size' and 'truncate' arguments... ... ok

======================================================================
FAIL: test_codon_tables (__main__.TestTestseq)
Testing codon sets...
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test_testseq.py", line 87, in test_codon_tables
    self.assertFalse(seq[0] == "M")
AssertionError: True is not false

----------------------------------------------------------------------
Ran 6 tests in 0.008s

FAILED (failures=1)

Adil-Iqbal added 3 commits July 3, 2017 11:23

Reverted seeding. Table argument now accepts strings and objects. Upd…

0010ccf

…ated docstrings.

Upgrade compatibility with future versions of Python.

2058899

Minor code clean-up

8451201

peterjc reviewed Jul 3, 2017

View reviewed changes

1. Remove 'is_outside_bio' function from run_tests.py.

2d9e5f5

2. Remove reference to 'ModuleNotFoundError' 3. Simplify 'manually_import' function for only python 2.7

peterjc reviewed Jul 3, 2017

View reviewed changes

Use sys.version_info.

9f38c56

picrin reviewed Jul 6, 2017

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add 'testseq' function #1306

Add 'testseq' function #1306

Adil-Iqbal commented Jul 3, 2017 •

edited

Loading

peterjc Jul 3, 2017

Adil-Iqbal Jul 3, 2017

peterjc Jul 3, 2017

Adil-Iqbal Jul 3, 2017

peterjc Jul 3, 2017

peterjc Jul 3, 2017

codecov bot commented Jul 3, 2017 •

edited

Loading

Adil-Iqbal commented Jul 3, 2017

peterjc Jul 3, 2017

Adil-Iqbal Jul 4, 2017

peterjc commented Jul 4, 2017 •

edited

Loading

picrin Jul 6, 2017

peterjc Jul 7, 2017

peterjc commented Jul 7, 2017

Add 'testseq' function #1306

Are you sure you want to change the base?

Add 'testseq' function #1306

Conversation

Adil-Iqbal commented Jul 3, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jul 3, 2017 • edited Loading

Codecov Report

Adil-Iqbal commented Jul 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterjc commented Jul 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterjc commented Jul 7, 2017

Adil-Iqbal commented Jul 3, 2017 •

edited

Loading

codecov bot commented Jul 3, 2017 •

edited

Loading

peterjc commented Jul 4, 2017 •

edited

Loading