Code review comment for lp:~ricardokirkner/configglue/from-future-remove-configparser

=== modified file 'configglue/_compat.py'
--- configglue/_compat.py 2013-05-27 00:56:38 +0000
+++ configglue/_compat.py 2013-07-09 20:10:35 +0000
> @@ -4,12 +4,109 @@
> PY2 = sys.version_info[0] == 2
>
> if not PY2:

I generally like to do positive conditionals, since negative conditionals are
more difficult to reason about. E.g. as currently written it takes longer to
realize that the if-stanza is for Python 3 and the else stanza is for Python
2.

=== modified file 'configglue/glue.py'
--- configglue/glue.py 2013-05-25 15:21:04 +0000
+++ configglue/glue.py 2013-07-09 20:10:35 +0000
> @@ -16,13 +16,10 @@
>
> import os
> import sys
> -from configparser import (
> - NoOptionError,
> - NoSectionError,
> -)

You could probably change this to:

from ._compat.configparser import (
    NoOptionError,
    NoSectionError,
    )

and then none of the except clauses would need to change.

> from optparse import OptionParser
> from collections import namedtuple
>
> +from ._compat import configparser
> from .parser import SchemaConfigParser
>
>
> @@ -68,7 +65,7 @@
> kwargs['help'] = option.help
> try:
> kwargs['default'] = parser.get(section.name, option.name)
> - except (NoSectionError, NoOptionError):
> + except (configparser.NoSectionError, configparser.NoOptionError):
> pass
> kwargs['action'] = option.action
> args = ['--' + long_name(option)]
> @@ -92,7 +89,7 @@
> op_value = getattr(options, opt_name(option))
> try:
> parser_value = parser.get(section.name, option.name)
> - except (NoSectionError, NoOptionError):
> + except (configparser.NoSectionError, configparser.NoOptionError):
> parser_value = None
> env_value = os.environ.get("CONFIGGLUE_{0}".format(
> long_name(option).upper()))

=== modified file 'configglue/parser.py'
--- configglue/parser.py 2013-07-08 20:10:06 +0000
+++ configglue/parser.py 2013-07-09 20:10:35 +0000
> @@ -21,16 +21,9 @@
> import os
> import re
>
> -from configparser import (
> - DEFAULTSECT,
> - SafeConfigParser as BaseConfigParser,
> - InterpolationMissingOptionError,
> - NoOptionError,
> - NoSectionError,
> -)

Similarly, this could probably be

from ._compat.configparser import (
    ...
    )

and less of the following code would need to change.

=== modified file 'configglue/schema.py'
--- configglue/schema.py 2013-05-26 19:07:46 +0000
+++ configglue/schema.py 2013-07-09 20:10:35 +0000
> @@ -16,14 +16,11 @@
> from __future__ import unicode_literals
>
> import json
> -from configparser import (
> - NoSectionError,
> - NoOptionError,
> -)

Similarly, here.

> from copy import deepcopy
> from inspect import getmembers
>
> -from configglue._compat import text_type, string_types
> +from ._compat import configparser, text_type, string_types
> +
>
>
> __all__ = [
> @@ -177,7 +174,7 @@
> """Return a Section by name"""
> section = self._sections.get(name)
> if section is None:
> - raise NoSectionError(name)
> + raise configparser.NoSectionError(name)
> return section
>
> def sections(self):
> @@ -248,7 +245,7 @@
> """Return a Option by name"""
> opt = getattr(self, name, None)
> if opt is None:
> - raise NoOptionError(name, self.name)
> + raise configparser.NoOptionError(name, self.name)
> return opt
>
> def options(self):

=== modified file 'configglue/tests/inischema/test_attributed.py'
--- configglue/tests/inischema/test_attributed.py 2013-05-26 16:15:48 +0000
+++ configglue/tests/inischema/test_attributed.py 2013-07-09 20:10:35 +0000
> @@ -19,9 +19,9 @@
> # runner's output, so pylint: disable-msg=C0111
>
> import unittest
> -from configparser import RawConfigParser

...and here. You get the idea, so I'll stop pointing out the same suggestion
with the following modules. :)

> from io import StringIO
>
> +from configglue._compat import configparser
> from configglue.inischema.attributed import AttributedConfigParser
>
>
> @@ -44,7 +44,7 @@
> class TestAttributed(BaseTest):
> """ pretty basic tests of AttributedConfigParser """
> def test_config_before_parsing_is_plain(self):
> - rawConfig = RawConfigParser()
> + rawConfig = configparser.RawConfigParser()
> rawConfig.readfp(StringIO(self.config_string))
> self.assertEqual([(section, sorted(self.config.items(section)))
> for section in self.config.sections()],

Other than that, and the removal of configparser from tox.ini, this looks
pretty good. I have an updated Saucy package branch ready to go as soon as
this is up on PyPI. Thanks for working on this!