Fix argparse integration requiring progressbar

[blochberger]

I split this pull request into two separate commits.

First, I separated the integrations into a module per integration. Now you can import only a specific integration:

from bitmath.integrations.argparse import BitmathType

This change is non-invasive, as the old import command will still work (and still lead to the error described in #86 ). Hence, no changes to tests are required.

Second, I added a reqression test for this fix, although this could probably be done better. One should probably use different tests for different integrations.

Maybe in the future it could be done like requests does it: enabling integrations with extra_requirements, so that you can install bitmath with progressbar integration via:

pip install bitmath[progressbar]

[tbielawa]

WOW! This is quite a well thought out PR. This is one of the things I love about open source projects. Thank you for submitting this.

I have errands to run this morning. I will give this a thorough review today. It looks great from what I saw already, I'm excited to try this out.

A note about the progress bar stuff. Don't worry too much about it because the Fedora package universe has deprecated the old abandoned progressbar package. I will probably have to disable it for a while until the bitmath integration can be refactored to work with progressbar2.

However, if you are interested in getting involved with that, there's more details in these two bugs:

• RFE: Use progressbar2 #85
• https://bugzilla.redhat.com/show_bug.cgi?id=1654747

I haven't looked too closely into this yet. So it's entirely possible that it's a simple fix and change of dependencies.

As for that travis failure. Uh. I am surprised.

ERROR: python-coveralls 2.9.1 has requirement coverage==4.0.3, but you'll have coverage 4.5.3 which is incompatible.

I wonder what's locking it down to that strange specific level... Oh well. More fun mysteries to unravel later.

Again, thank you for contributing. Stuff like this is what makes maintaining an open source project worth the effort :-)

[blochberger]

The Travis error you quoted occurs in the successful builds as well. It probably can be fixed by setting the coverage version explicitly in requirements.txt and requirements-py3.txt: coverage==4.0.3.

I did not look into the Python 2 build failure, as I have not used Python 2 for a very long time.

I am not really interested in the progressbar stuff but I am happy to take a look into #74 once this PR lands (as it might require an external dependency as well).

[tbielawa]

Hi again @blochberger

I was scratching my head this morning trying to pin down two errors that stood out to me.

    raise argparse.ArgumentTypeError("'%s' can not be parsed into a valid bitmath object" %
AttributeError: 'module' object has no attribute 'ArgumentTypeError'

and

  File "/home/travis/build/tbielawa/bitmath/bitmath/integrations/progressbar.py", line 2, in <module>
    import progressbar.widgets
ImportError: No module named widgets

it made no sense at all that argparse wouldn't have the ArgumentTypeError exception available. Likewise with progressbar.widgets. It's trivial to verify this oneself. I began to formulate a hypothesis so I did some digging and added some debugging.

Make sure nosetests doesn't hide standard out, add a print statement to bitmath.integrations.argparse after it imports argparse from stdlib

diff --git a/Makefile b/Makefile
index 6267447..7d32f3e 100644
--- a/Makefile
+++ b/Makefile
@@ -210,10 +210,10 @@ ci-unittests2:
 	@echo "# Running Unit Tests in virtualenv"
 	@echo "# Using python: $(shell ./bitmathenv2/bin/python --version 2>&1)"
 	@echo "#############################################"
-	. $(NAME)env2/bin/activate && export PYVER=PY2X && nosetests -v --with-coverage --cover-html --cover-min-percentage=90 --cover-package=bitmath tests/
+	. $(NAME)env2/bin/activate && export PYVER=PY2X && nosetests --nocapture -v --with-coverage --cover-html --cover-min-percentage=90 --cover-package=bitmath tests/
 	@echo "Testing argparse integration without progressbar dependency (#86)"
 	. $(NAME)env2/bin/activate && pip uninstall -y progressbar231
-	. $(NAME)env2/bin/activate && export PYVER=PY2X && nosetests -v --with-coverage --cover-html --cover-min-percentage=90 --cover-package=bitmath tests/test_argparse_type.py
+	. $(NAME)env2/bin/activate && export PYVER=PY2X && nosetests -v --nocapture --with-coverage --cover-html --cover-min-percentage=90 --cover-package=bitmath tests/test_argparse_type.py
 
 ci-list-deps2:
 	@echo "#############################################"
diff --git a/bitmath/integrations/argparse.py b/bitmath/integrations/argparse.py
index bd5fdb2..01958ad 100644
--- a/bitmath/integrations/argparse.py
+++ b/bitmath/integrations/argparse.py
@@ -26,7 +26,7 @@
 
 import bitmath
 import argparse
-
+print argparse.__file__
 
 def BitmathType(bmstring):
     """An 'argument type' for integrations with the argparse module.

And then I ran ci-unittests2 again I received confirmation:

# Running Unit Tests in virtualenv
# Using python: Python 2.7.15
#############################################
. bitmathenv2/bin/activate && export PYVER=PY2X && nosetests --nocapture -v --with-coverage --cover-html --cover-min-percentage=90 --cover-package=bitmath tests/
>>>> THIS HERE >>>> /home/tbielawa/Projects/bitmath/bitmath/integrations/argparse.py
Argparse: BitmathType - Unquoted values separated from their units are detected ... ERROR
...

When integrations/argparse.py runs import argparse it is actually importing itself, not argparse from standard library. Similarly with progressbar it is emitting the No module named widgets error because bitmath/integrations/progressbar.py doesn't have widgets anything inside of itself.

I have a suggestion which I tested and appeared to work just now:

• Prefix the files under bitmath/integrations with bm

        renamed:    bitmath/integrations/argparse.py -> bitmath/integrations/bmargparse.py
        renamed:    bitmath/integrations/progressbar.py -> bitmath/integrations/bmprogressbar.py

• and update import/test/doc references

--- a/bitmath/integrations/__init__.py
+++ b/bitmath/integrations/__init__.py
@@ -25,10 +25,10 @@

-from .argparse import BitmathType
+from .bmargparse import BitmathType
 
 try:
-    from .progressbar import BitmathFileTransferSpeed
+    from .bmprogressbar import BitmathFileTransferSpeed

diff --git a/tests/test_argparse_type.py b/tests/test_argparse_type.py
index 6cfba41..14ec4f6 100644
--- a/tests/test_argparse_type.py
+++ b/tests/test_argparse_type.py
@@ -30,7 +30,7 @@ Test the argparse 'BitmathType' integration
 
 from . import TestCase
 import bitmath
-from bitmath.integrations.argparse import BitmathType
+from bitmath.integrations.bmargparse import BitmathType
 import argparse
 import shlex
 
diff --git a/tests/test_progressbar.py b/tests/test_progressbar.py
index 148eeaf..304e14e 100644
--- a/tests/test_progressbar.py
+++ b/tests/test_progressbar.py
@@ -30,7 +30,7 @@ Test the progressbar 'FileTransferSpeed' integration
 
 from . import TestCase
 import bitmath
-from bitmath.integrations.progressbar import BitmathFileTransferSpeed
+from bitmath.integrations.bmprogressbar import BitmathFileTransferSpeed
 import mock
 import progressbar

After I did that (and updated requirements*.txt) all the tests were passing on Python 2 and 3. If you can make those changes and update the requirements like this then this PR will be ready to merge!

diff --git a/requirements-py3.txt b/requirements-py3.txt
index d239691..a777639 100644
--- a/requirements-py3.txt
+++ b/requirements-py3.txt
@@ -1,5 +1,5 @@
+coverage==4.0.3
 python-coveralls
-coverage
 mock
 nose
 nose-cover3
diff --git a/requirements.txt b/requirements.txt
index 863b3e7..a83f10c 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,5 +1,5 @@
+coverage==4.0.3
 python-coveralls
-coverage
 mock
 nose
 pyflakes

Thanks for submitting your bug report and this PR, Max. And more importantly, thanks for your patience while I got around to getting back to you.

[blochberger]

Thanks for figuring it out, @tbielawa. Looks good now.

[tbielawa]

@blochberger thank you so much for your contribution! This looks great.