Add "testseq.py" to Scripts

[Adil-Iqbal]

Add "testseq" function.

[codecov]

Codecov Report

Merging #1269 into master will increase coverage by 0.03%.
The diff coverage is 94.44%.

@@            Coverage Diff             @@
##           master    #1269      +/-   ##
==========================================
+ Coverage    84.5%   84.53%   +0.03%     
==========================================
  Files         317      318       +1     
  Lines       48702    48900     +198     
==========================================
+ Hits        41156    41339     +183     
- Misses       7546     7561      +15

Impacted Files	Coverage Δ
Scripts/testseq.py	94.44% <94.44%> (ø)
Bio/GA/Evolver.py	54.54% <0%> (-18.19%)	⬇️
Bio/Phylo/PhyloXML.py	81.39% <0%> (ø)	⬆️
Bio/SCOP/__init__.py	61.42% <0%> (ø)	⬆️

---

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 63cc7bc...23de4ac. Read the comment docs.

[peterjc]

Rather than assuming / as a path separator for splitting (which will break on Windows), use os.path.split(...) and os.path.join(...) for the path manipulations. Or just assume that it will be run from the Tests/ folder and hard-code the relative path ../Scripts/ (which should work on Windows which will accept the other slashes as input)?

[andrewguy]

I'm still concerned that the random number generation is a little unconventional, and more confusing than it needs to be.

I understand your concern that the code needs to be intuitive for those unfamiliar with coding in general, but an equally valid concern here is code readability for anyone who reads the source code. Random number generation should be straightforward, and there are fairly well established ways of doing it.

As written, you're seeding a random number generator with the output from another random number generator, every time you make a call to random().

Can we maybe get another set of eyes on this? This is what I imagine it should look like, but happy for people to disagree - https://github.com/Adil-Iqbal/Personal-Projects/pull/1/files

[Adil-Iqbal]

That's a very good point. You're right.

If this code is to be used in an educational capacity, no sense trying to reinforce concepts that would have to be unlearned. I'll see what I can do about reverting the seeding method to a former state. Though, you may have to wait a bit. I'm trying to get this unit test to where it needs to be. (Which I'm sure is just a matter of hammering out the details.)

Just an FYI, I'll need to change the doctests for better version compatibility.

[Adil-Iqbal]

The following was copy/pasted from the mailing list:

TL:DR - Game plan: (1) Resolve unittest importing issue. (2) Revert testseq seeding. (3) Revisit testseq doctests. (4) Update unittest for reverted seeding. (2) Thank you for your patience.

Hello all,

I'd like to let everyone know where 'testseq' is in development. You can find the code for it here: https://github.com/Adil-Iqbal/Personal-Projects/tree/master/Test%20Sequence

The biggest issue (unexpectedly) was importing the 'testseq' module into the 'test_testseq' unit test. Absolute and relative importing did not work since the 'Scripts' folder is not a part of the 'Bio' package. After trying out several solutions, the one that looks the most promising is using the "importlib" module. The issue is not yet resolved but I am confident that it is just a matter of working out the details.

I've gotten the chance to tackle part of another big issue, which is python version and operating system compatibility. Since the 'testseq' function uses randomization, its output can be different across versions and platforms. This poses a problem for code testing. Luckily there are a few work-arounds that I've implemented in the unit test which should work fine. I will also be revisiting the 'testseq' doctests to ensure they are also compatible, though I have not yet run into any issues with that on Travis CI. (Peter warned me about this. ^_^)

Today, an argument was presented for reverting the seeding implementation, which was that user intuition should preferably be developed with respect to best practices. Unless anyone has a reasonable rebuttal, I will probably revert the seeding to an earlier state. Though, that will have to wait until after I've resolved the importing issue.

Also, I'd like to thank you for being patient with me. I would've completed this sooner, but I have other obligations as well.

Warmest regards,
Adil

[peterjc]

Please restore this blank line which is a deliberate part of the docstring for private function _have_bug17666 - see https://www.python.org/dev/peps/pep-0257/#multi-line-docstrings

[peterjc]

Travis is failing on the style checks before ever running the tests:

$ flake8 Tests/
Tests/run_tests.py:1:1: D205 1 blank line required between summary line and description
Tests/run_tests.py:171:1: D205 1 blank line required between summary line and description
Tests/run_tests.py:323:1: D205 1 blank line required between summary line and description

You can install flake8 locally, see https://github.com/biopython/biopython/blob/master/CONTRIBUTING.rst

[andrewguy]

Your latest build is failing with Python 2.7 because you are doing division, and haven't added from __future__ import division. Python 2.7 division defaults to floor division, whereas Python 3+ is floating point division.

I would really recommend testing what you can on your local machine before pushing changes to github.

You can use either pip's virtualenv, or conda's env to set up self-contained environments for testing (if you haven't already done so). You can set up a Python 2.7 environment this way, and make sure your build passes on both 2.7 and 3+.

Peter's suggestion of running Flake 8 locally is also a very good one.

[Adil-Iqbal]

Thanks a lot guys. I really appreciate it. I've got flake8 up and running locally. I'll try to get virtualenv going too. Definitely a bit heavy on the commits. Sorry about that.

[peterjc]

You have accidentally remove this (and two more) blank line from the module's docstring.

[Adil-Iqbal]

Reverted. Will include changes in next commit.

[peterjc]

I wonder if the complicated import tricks you are trying in order to be able to do from Scripts import testseq or similar could be avoided by including Scripts/ in sys.path in the same way that run_tests.py ensures our complied C modules can be imported from the build/ subdirectory?

[Adil-Iqbal]

That method will certainly succeed in importing the module. The issue is that once the importing is complete, the doctests are run. And the doctest also contain an import statement. The doctests cause an error in 2.7 because they are subject to the same importing limitations as "run_tests.py". I will now describe these limitations below:

Prior to 3.3, hierarchical importing (with "dot" notation) required a directory to be initialized with a "__init__.py" file. With the introduction of 3.3, hierarchical importing was generalized to all directories, regardless of initialization. This is why my commits were passing all tests except 2.7.

Since we can't initialize folders carelessly, its best to remove the non-Bio modules from the "run_tests.py" file just prior to version 3.3. All versions of python after-and-including 3.3 don't require an extra "__init__.py" file, so the doctests will run fine on 3.3, 3.4, 3.5, and 3.6. That's why the most recent build passed the Travis CI test.

I agree that my import methods were becoming overly-complicated. In the most recent build, I've eliminated the complicated code in favor of just deleting non-Bio files. The only complicated code left is limited to just the unit test ("test_testseq.py").

Thankfully, the unit test for 'testseq' runs all of the same tests that are in the doctests, so we can be confident that 'testseq' is compatible with all versions of python. In the future, if "biopython" decides to stop supporting python 2.7, I would be happy to remove the last bit of complicated code.

TL:DR - In 2.7 only: Modules outside 'Bio' package can be imported from "run_tests.py", however, their doctests will likely fail because they must also contain import statements. It's best to remove them from doctesting in earlier versions of python and allow the unit tests to confirm compatibility. (Only for non-Bio files in only 2.7).

[peterjc]

I'm pretty sure you can remove these lines from testseq.py and the doctest will still work as the classes etc are defined in the current file so already present in the local scope:

>>> from Scripts.testseq import testseq

[Adil-Iqbal]

Closing pull request.