followup -Hi - refactoring Option creation - making a factory

* added files

    lib/option_types.py
    test/CleanOption.py
    test/CleanOption2.py
    test/CleanOptionTest.py
    test/TraditionalOption.py
    test/__init__.py

* modified files

--- orig/examples/required_2.py
+++ mod/examples/required_2.py
@@ -8,7 +8,10 @@
 import optik
 
 class Option (optik.Option):
-    ATTRS = optik.Option.ATTRS + ['required']
+    def __init__(self, *args, **kwargs):
+        self.required=kwargs.pop('required', None)
+        optik.Option.__init__(self,*args, **kwargs)
+        self._check_required()
 
     def _check_required (self):
         if self.required and not self.takes_value():
@@ -16,9 +19,6 @@
                 "required flag set for option that doesn't take a value",
                  self)
 
-    # Make sure _check_required() is called from the constructor!
-    CHECK_METHODS = optik.Option.CHECK_METHODS + [_check_required]
-
     def process (self, opt, value, values, parser):
         optik.Option.process(self, opt, value, values, parser)
         parser.option_seen[self] = 1


--- orig/extending.txt
+++ mod/extending.txt
@@ -14,29 +14,25 @@
 Adding new types
 ----------------
 
-To add new types, you need to define your own subclass of Optik's Option
-class.  This class has a couple of attributes that define Optik's types:
-TYPES and TYPE_CHECKER.
-
-TYPES is a tuple of type names; in your subclass, simply define a new
-tuple TYPES that builds on the standard one.
-
-TYPE_CHECKER is a dictionary mapping type names to type-checking
-functions.  A type-checking function has the following signature::
-
-  def check_foo (option : Option, opt : string, value : string)
-                -> foo
-
-You can name it whatever you like, and make it return any type you
-like.  The value returned by a type-checking function will wind up in
-the OptionValues instance returned by OptionParser.parse_args(), or be
-passed to callbacks as the 'value' parameter.
-
-Your type-checking function should raise OptionValueError if it
-encounters any problems.  OptionValueError takes a single string
-argument, which is passed as-is to OptionParser's error() method, which
-in turn prepends the program name and the string "error:" and prints
-everything to stderr before terminating the process.
+The was an API (Now deprecated) where you override Option.TYPES and
+Option.TYPE_CHECKER. This is now not needed, and recommended against.
+To add a new type to the list of types automatically created, use
+OptionTypes.add_type("name", YourType)
+
+To add new types, you need to define your own subclass (YourType above)
+of Optik's Option class.  You need to override check_value 
+(self, opt, value) in your subclass.
+
+Your check_value method can return any type you like.  The value returned 
+by a type-checking function will wind up in the OptionValues instance
+returned by OptionParser.parse_args(), or be passed to callbacks as the
+'value' parameter.
+
+Your check_value method should raise OptionValueError if it encounters
+any problems.  OptionValueError takes a single string argument, which is
+passed as-is to OptionParser's error() method, which in turn prepends the
+program name and the string "error:" and prints everything to stderr before
+terminating the process.
 
 Here's a silly example that demonstrates adding a "complex" option type
 to parse Python-style complex numbers on the command line.  (This is
@@ -46,44 +42,51 @@
 First, the necessary imports::
 
   from copy import copy
-  from optik import Option, OptionValueError
+  from optik import Option, OptionValueError, OptionTypes
 
-You need to define your type-checker first, since it's referred to later
-(in the TYPE_CHECKER class attribute of your Option subclass)::
+Secondly we define your type::
 
-  def check_complex (option, opt, value):
-      try:
-          return complex(value)
-      except ValueError:
-          raise OptionValueError(
-              "option %s: invalid complex value: %r" % (opt, value))
-
-Finally, the Option subclass::
-
-  class MyOption (Option):
-      TYPES = Option.TYPES + ("complex",)
-      TYPE_CHECKER = copy(Option.TYPE_CHECKER)
-      TYPE_CHECKER["complex"] = check_complex
-
-(If we didn't make a copy() of Option.TYPE_CHECKER, we would end up
-modifying the TYPE_CHECKER attribute of Optik's Option class.  This
-being Python, nothing stops you from doing that except good manners and
-common sense.)
+class ComplexOption (Option):
+    def check_value (self, opt, value):
+        try:
+            return complex(value)
+        except ValueError:
+	    raise OptionValueError(
+	        "option %s: invalid complex value: %r" % (opt, value))
+
+Lastly, we (optionally, see below) associate "complex" with the 
+ComplexOption type; we can do this two ways. We can make the type
+globally available::
+  OptionTypes.add_type("complex", ComplexOption)
+or we can create our own set of Types:
+  MyOptionTypes=copy(OptionTypes)
+  MyOptionTypes.add_type("complex", ComplexOption)
 
 That's it!  Now you can write a script that uses the new option type
 just like any other Optik-based script, except you have to instruct your
-OptionParser to use MyOption instead of Option::
+OptionParser to use MyOptionTypes instead of Option (assuming you did not
+take the 'easy' way and make the type global)::
 
-  parser = OptionParser(option_class=MyOption)
+  parser = OptionParser(option_class=MyOptionTypes)
   parser.add_option("-c", action="store", type="complex", dest="c")
 
 Alternately, you can build your own option list and pass it to
 OptionParser; if you don't use add_option() in the above way,
 you don't need to tell OptionParser which option class to use::
 
-  option_list = [MyOption("-c", action="store", type="complex", dest="c")]
+  option_list = [ComplexOption("-c", action="store", dest="c")]
   parser = OptionParser(option_list=option_list)
 
+If you want to avoid all global type lists and building an option
+list, you can also simply use your type as an argument to add_option:
+  parser = OptionParser()
+  parser.add_option(ComplexOption("-c", action="store", dest="c"))
+
+If you want to avoid all global type lists and building an option
+list, you can also simply use your type as an argument to add_option:
+  parser = OptionParser()
+  parser.add_option(ComplexOption("-c", action="store", dest="c"))
+
 
 Adding new actions
 ------------------


--- orig/lib/__init__.py
+++ mod/lib/__init__.py
@@ -17,6 +17,7 @@
 
 # Re-import these for convenience
 from optik.option import Option
+from optik.option_types import OptionTypes
 from optik.option_parser import *
 from optik.help import *
 from optik.errors import *
@@ -32,4 +33,7 @@
 # preferred way to instantiate Options is indirectly, via make_option(),
 # which will become a factory function when there are many Option
 # classes.
+# NOTE: this should not be exposed to end users, as it isn't a factory,
+# and the only user - the test suite - isn't compatible with a 
+# factory that has a default..
 make_option = Option


--- orig/lib/option.py
+++ mod/lib/option.py
@@ -13,6 +13,7 @@
 import sys
 import types
 from optik.errors import OptionError, OptionValueError
+from optik.option_types import OptionTypes
 
 __all__ = ['Option']
 
@@ -22,29 +23,6 @@
 except NameError:
     (True, False) = (1, 0)
 
-_builtin_cvt = { "int" : (int, "integer"),
-                 "long" : (long, "long integer"),
-                 "float" : (float, "floating-point"),
-                 "complex" : (complex, "complex") }
-
-def check_builtin (option, opt, value):
-    (cvt, what) = _builtin_cvt[option.type]
-    try:
-        return cvt(value)
-    except ValueError:
-        raise OptionValueError(
-            #"%s: invalid %s argument %r" % (opt, what, value))
-            "option %s: invalid %s value: %r" % (opt, what, value))
-
-def check_choice(option, opt, value):
-    if value in option.choices:
-        return value
-    else:
-        choices = ", ".join(map(repr, option.choices))
-        raise OptionValueError(
-            "option %s: invalid choice: %r (choose from %s)"
-            % (opt, value, choices))
-
 # Not supplying a default is different from a default of None,
 # so we need an explicit "not supplied" value.
 NO_DEFAULT = "NO"+"DEFAULT"
@@ -113,32 +91,9 @@
                      "append",
                      "callback")
 
-    # The set of known types for option parsers.  Again, listed here for
-    # constructor argument validation.
-    TYPES = ("string", "int", "long", "float", "complex", "choice")
-
-    # Dictionary of argument checking functions, which convert and
-    # validate option arguments according to the option type.
-    #
-    # Signature of checking functions is:
-    #   check(option : Option, opt : string, value : string) -> any
-    # where
-    #   option is the Option instance calling the checker
-    #   opt is the actual option seen on the command-line
-    #     (eg. "-a", "--file")
-    #   value is the option argument seen on the command-line
-    #
-    # The return value should be in the appropriate Python type
-    # for option.type -- eg. an integer if option.type == "int".
-    #
-    # If no checker is defined for a type, arguments will be
-    # unchecked and remain strings.
-    TYPE_CHECKER = { "int"    : check_builtin,
-                     "long"   : check_builtin,
-                     "float"  : check_builtin,
-                     "complex"  : check_builtin,
-                     "choice" : check_choice,
-                   }
+    # Legacy direct-internal modification API.
+    TYPES = OptionTypes.TYPES
+    TYPE_CHECKER = OptionTypes.TYPE_CHECKER
 
 
     # CHECK_METHODS is a list of unbound method objects; they are called
@@ -239,7 +194,7 @@
                     # No type given?  "string" is the most sensible default.
                     self.type = "string"
         else:
-            if self.type not in self.TYPES:
+            if self.type not in self.TYPES and not OptionTypes.valid_type(self):
                 raise OptionError("invalid option type: %r" % self.type, self)
             if self.action not in self.TYPED_ACTIONS:
                 raise OptionError(


--- orig/lib/option_parser.py
+++ mod/lib/option_parser.py
@@ -15,6 +15,7 @@
 from optik.option import Option, NO_DEFAULT
 from optik.help import IndentedHelpFormatter
 from optik.errors import OptionConflictError, OptionValueError, BadOptionError
+from optik.option_types import OptionTypes
 
 __all__ = ['SUPPRESS_HELP', 'SUPPRESS_USAGE',
            'Values', 'OptionContainer', 'OptionGroup', 'OptionParser']
@@ -184,7 +185,7 @@
            add_option(opt_str, ..., kwarg=val, ...)
         """
         if type(args[0]) is types.StringType:
-            option = self.option_class(*args, **kwargs)
+            option = OptionTypes.factory(self.option_class, *args, **kwargs)
         elif len(args) == 1 and not kwargs:
             option = args[0]
             if not isinstance(option, Option):


--- orig/merge
+++ mod/merge
@@ -124,7 +124,8 @@
     # last because it depends on all the others.
     modules = [("errors",        "class OptikError"),
                ("help",          "class HelpFormatter"),
-               ("option",        "_builtin_cvt"),
+	       ("option_types",  "_builtin_cvt"),
+               ("option",        "# Not supplying"),
                ("option_parser", "SUPPRESS_HELP")]
     if options.compatible:
         modules[2] = ("option", "# Do the right thing with boolean values")


--- orig/test/test_optik.py
+++ mod/test/test_optik.py
@@ -17,13 +17,19 @@
 from pprint import pprint
 from test import test_support
 
-test_dir = os.path.dirname(sys.argv[0])
-build_dir = os.path.join(test_dir, os.pardir, "build", "lib")
-if os.path.isdir(build_dir):
-    sys.path.insert(0, build_dir)
-else:
-    sys.exit("you must run 'python setup.py build' before "
-             "running the test suite")
+def tryBuildDir(a_dir):
+    """try adding '${a_dir}/../build/lib' to the path"""
+    build_dir = os.path.join(a_dir, os.pardir, "build", "lib")
+    if os.path.isdir(build_dir):
+        sys.path.insert(0, build_dir)
+        return True
+    else:
+        return False
+
+if not tryBuildDir(os.path.dirname(sys.argv[0])):
+    if not tryBuildDir(os.path.abspath(os.path.curdir)):
+        sys.exit("you must run 'python setup.py build' before "
+                 "running the test suite")
 
 from optik import make_option, Option, IndentedHelpFormatter, \
      TitledHelpFormatter, OptionParser, OptionContainer, OptionGroup, \
@@ -1263,4 +1269,8 @@
     test_support.run_suite(suite())
 
 if __name__ == '__main__':
+    #TODO don't use unittest.main, rather test_main? Anyway, fix this to 
+    # invoke both tests, but the main one first, and CleanOptionTest later
+    import CleanOptionTest
+    test_support.run_suite(CleanOptionTest.parserTestSuite)
     unittest.main()


--- /dev/null	2004-04-29 15:50:28.000000000 +1000
+++ lib/option_types.py	2004-05-09 15:30:39.000000000 +1000
@@ -0,0 +1,94 @@
+"""optik.option_types
+
+Defines the Registry of known Option classes for the OptionParser.add_option convenience function.
+"""
+
+__revision__ = "$Id: garbage $"
+
+# Copyright (c) 2001-2003 Gregory P. Ward.  All rights reserved.
+# Copyright (c) 2004 Robert Collins <[email protected]>.  All rights reserved.
+# See the README.txt distributed with Optik for licensing terms.
+
+# created 20040509 (extracted from option.py)
+
+import sys
+import types
+from optik.errors import OptionError, OptionValueError
+
+_builtin_cvt = { "int" : (int, "integer"),
+                 "long" : (long, "long integer"),
+                 "float" : (float, "floating-point"),
+                 "complex" : (complex, "complex") }
+
+def check_builtin (option, opt, value):
+    (cvt, what) = _builtin_cvt[option.type]
+    try:
+        return cvt(value)
+    except ValueError:
+        raise OptionValueError(
+            #"%s: invalid %s argument %r" % (opt, what, value))
+            "option %s: invalid %s value: %r" % (opt, what, value))
+
+def check_choice(option, opt, value):
+    if value in option.choices:
+        return value
+    else:
+        choices = ", ".join(map(repr, option.choices))
+        raise OptionValueError(
+            "option %s: invalid choice: %r (choose from %s)"
+            % (opt, value, choices))
+
+class OptionTypes(object):
+    """I am an abstract factory for the creation of Ooptions"""
+    # The set of known types for option parsers.  Again, listed here for
+    # constructor argument validation.
+    TYPES = ("string", "int", "long", "float", "complex", "choice")
+    
+    # Dictionary of argument checking functions, which convert and
+    # validate option arguments according to the option type.
+    #
+    # Signature of checking functions is:
+    #   check(option : Option, opt : string, value : string) -> any
+    # where
+    #   option is the Option instance calling the checker
+    #   opt is the actual option seen on the command-line
+    #     (eg. "-a", "--file")
+    #   value is the option argument seen on the command-line
+    #
+    # The return value should be in the appropriate Python type
+    # for option.type -- eg. an integer if option.type == "int".
+    #
+    # If no checker is defined for a type, arguments will be
+    # unchecked and remain strings.
+    TYPE_CHECKER = { "int"    : check_builtin,
+                     "long"   : check_builtin,
+                     "float"  : check_builtin,
+                     "complex"  : check_builtin,
+                     "choice" : check_choice,
+                   }
+
+    newTypes={}
+    """Dictionary mapping type names to classes"""
+
+    def add_type(option_class_name, option_class):
+        """register a type constructor into the the global registry of types. """
+        if option_class_name in OptionTypes.TYPES or option_class_name in OptionTypes.newTypes:
+            raise KeyError("type already present")
+        OptionTypes.newTypes[option_class_name]=option_class
+    add_type=staticmethod(add_type)
+    def factory(default_type, *args, **kwargs):
+        """create an Option or a subclassed Option based on the arguments"""
+        typestr=kwargs.get('type',None)
+        if typestr:
+            type=OptionTypes.newTypes.get(typestr,None)
+            if type:
+                return type(*args, **kwargs)
+        return default_type(*args, **kwargs)
+    factory=staticmethod(factory)
+    def valid_type(someOption):
+        """return true if someOption is a type in newTypes"""
+        for type in OptionTypes.newTypes.items():
+            if isinstance(someOption, type[1]):
+                return True
+        return False
+    valid_type=staticmethod(valid_type)
--- /dev/null	2004-04-29 15:50:28.000000000 +1000
+++ test/__init__.py	2004-05-08 23:05:15.000000000 +1000
@@ -0,0 +1,6 @@
+# Empty file. 
+# Copyright 2004 (c) Robert Collins <[email protected]>
+# licenced under the BSD 3 clause licence.
+
+
+#arch-tag: e51e469e-4e8e-4792-9163-402d65d2ecf7
--- /dev/null	2004-04-29 15:50:28.000000000 +1000
+++ test/CleanOption.py	2004-05-09 00:27:10.000000000 +1000
@@ -0,0 +1,19 @@
+# Copyright 2004 (c) Robert Collins <[email protected]>
+# licenced under the BSD 3 clause licence.
+
+from optik import Option, OptionValueError
+
+class CleanOption (Option):
+    def fail(self, opt, value):
+        raise OptionValueError("option %s: invalid clean value: %r" % (opt, value))
+
+    def check_value (self, opt, value):
+        try:
+            if value=="clean":
+                return "found"
+            else:
+                self.fail(opt, value)
+        except ValueError:
+            self.fail(opt, value)
+
+#arch-tag: 653cf43d-3a88-4b52-a38f-08a0de532bcb
--- /dev/null	2004-04-29 15:50:28.000000000 +1000
+++ test/CleanOption2.py	2004-05-09 00:26:49.000000000 +1000
@@ -0,0 +1,16 @@
+# Copyright 2004 (c) Johannes Berg <[email protected]>
+# licenced under the BSD 3 clause licence.
+
+from CleanOption import CleanOption
+
+class CleanOption2 (CleanOption):
+    def check_value (option, opt, value):
+        try:
+            if value=="clean2":
+                return "found"
+            else:
+                self.fail(opt, value)
+        except ValueError:
+            self.fail(opt, value)
+
+# arch-tag: 9c4ea467-f744-4a0b-9b9a-50c589e39543
--- /dev/null	2004-04-29 15:50:28.000000000 +1000
+++ test/CleanOptionTest.py	2004-05-09 15:45:10.000000000 +1000
@@ -0,0 +1,81 @@
+#!/usr/bin/env python
+# Copyright 2004 (c) Robert Collins <[email protected]>
+# licenced under the BSD 3 clause licence.
+ 
+import unittest
+import sys
+
+parserTestSuite = unittest.TestSuite()
+ 
+class CleanOptionTestCase(unittest.TestCase):
+    """I test the use of CleanOption and optik module"""
+    def testImport(self):
+        """Test that we can import our module"""
+        import CleanOption
+    def testTraditionExtend(self):
+        """Test that we haven't broken the traditional means of extension"""
+        from TraditionalOption import TraditionalOption
+        from optik import OptionParser
+        parser = OptionParser(option_class=TraditionalOption)
+        parser.add_option("-d", action="store", type="dirty", dest="d")
+        (options, args) = parser.parse_args(["-d", "dirty"])
+        self.assertEquals(options.__dict__, {'d':'laundry'})
+    def testParseWithOption(self):
+        """Test that we can use CleanOption"""
+        from CleanOption import CleanOption
+        from optik import OptionParser
+        parser = OptionParser()
+        parser.add_option(CleanOption("-c", "--clean", action="store", dest="c"))
+        (options, args) = parser.parse_args(["-c", "clean"])
+        self.assertEquals(options.__dict__, {'c':'found'})
+    def testParseWithOption2(self):
+        """Test that we can use a descendent of CleanOption with the same TYPE"""
+        from CleanOption2 import CleanOption2
+        from CleanOption import CleanOption
+        from optik import OptionParser
+        parser = OptionParser()
+        parser.add_option(CleanOption("-c", "--clean", action="store", dest="c"))
+        parser.add_option(CleanOption2("-2", "--clean2", action="store", dest="2"))
+        (options, args) = parser.parse_args(["-c", "clean", "-2", "clean2"])
+        self.assertEquals(options.__dict__, {'c':'found','2':'found'})
+    def testParseWithAddedType(self):
+        """Test that a registered new Option class type works"""
+        from CleanOption import CleanOption
+        from optik import OptionParser, OptionTypes
+        OptionTypes.add_type("clean", CleanOption)
+        parser = OptionParser()
+        parser.add_option("-c", "--clean", action="store", dest="c", type="clean")
+        (options, args) = parser.parse_args(["-c", "clean"])
+        self.assertEquals(options.__dict__, {'c':'found'})
+    def testHelp(self):
+        """Check that help works, not by testing '--help' though"""
+        from CleanOption2 import CleanOption2
+        from CleanOption import CleanOption
+        from optik import OptionParser
+        parser = OptionParser("usage: CleanOptionTest [options]")
+        parser.add_option(CleanOption("-c", "--clean", action="store", dest="c"))
+        parser.add_option(CleanOption2("-2", "--clean2", action="store", dest="2"))
+        help = parser.format_help()
+        self.assertEquals(help, 'usage: CleanOptionTest [options]\n\noptions:\n  -h, --help       show this help message and exit\n  -cC, --clean=C   \n  -22, --clean2=2  \n')
+        
+        
+parserTestSuite.addTest(CleanOptionTestCase("testImport"))
+parserTestSuite.addTest(CleanOptionTestCase("testTraditionExtend"))
+parserTestSuite.addTest(CleanOptionTestCase("testParseWithOption"))
+parserTestSuite.addTest(CleanOptionTestCase("testParseWithOption2"))
+parserTestSuite.addTest(CleanOptionTestCase("testParseWithAddedType"))
+parserTestSuite.addTest(CleanOptionTestCase("testHelp"))
+ 
+def suite():
+    return parserTestSuite
+ 
+def main(argv):
+    suite = parserTestSuite
+    runner=unittest.TextTestRunner(verbosity=2)
+    if not runner.run(suite).wasSuccessful(): return 1
+    return 0
+ 
+if __name__ == '__main__':
+    sys.exit(main(sys.argv))
+ 
+# arch-tag: 2c8b8dad-6513-4d1b-bc53-4de5f0cbe418
--- /dev/null	2004-04-29 15:50:28.000000000 +1000
+++ test/TraditionalOption.py	2004-05-09 00:49:13.000000000 +1000
@@ -0,0 +1,26 @@
+# Copyright 2004 (c) Robert Collins <[email protected]>
+# licenced under the BSD 3 clause licence.
+
+from copy import copy
+from optik import Option, OptionValueError
+
+# variation of old extending sample code
+
+def fail(opt, value):
+    raise OptionValueError("option %s: invalid dirty value: %r" % (opt, value))
+
+def check_dirty (option, opt, value):
+    try:
+        if value=="dirty":
+            return "laundry"
+        else:
+            fail(opt, value)
+    except ValueError:
+        fail(opt, value)
+
+class TraditionalOption (Option):
+    TYPES = Option.TYPES + ("dirty",)
+    TYPE_CHECKER = copy(Option.TYPE_CHECKER)
+    TYPE_CHECKER["dirty"] = check_dirty
+
+#arch-tag: 5d8dea1e-aa19-41e2-b048-af3f2f941b5f

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (GNU/Linux)

iD8DBQBAnsucI5+kQ8LJcoIRAnY0AJ9dE/AFsPdgihb1oPfFtneGYy+KMQCdFMKm
myn6dy+b55dsSw+iUOef6VY=
=MjOW
-----END PGP SIGNATURE-----