cleanup, incorporate changes from code review, and move merge and split stuff into freebase.experimental

git-svn-id: http://freebase-python.googlecode.com/svn/trunk@201 5914aa95-5b3a-0410-a3b5-7b719e7fe9b2

12 files changed, 325 insertions, 400 deletions

diff --git a/freebase/api/session.py b/freebase/api/session.py
index 7c6b98b..5d3b6a0 100644
--- a/freebase/api/session.py
+++ b/freebase/api/session.py
@@ -860,7 +860,8 @@ class HTTPMetawebSession(MetawebSession):
         """DEPRECATED: reconcile name to guid. For a more complete description,
         see http://www.freebase.com/view/en/dataserver_reconciliation
         
-        If interested, check out http://data.labs.freebase.com/recon/"""
+        If interested in a non-deprecated version,
+        check out http://data.labs.freebase.com/recon/"""
         
         service = '/dataserver/reconciliation'
         r = self._httpreq_json(service, 'GET', form={'name':name, 'types':','.join(etype)})
@@ -870,99 +871,6 @@ class HTTPMetawebSession(MetawebSession):
         #self._mqlresult(r)
         return r
 
-    ### SCHEMA MANIPULATION ###
-    # Object helpers
-    def create_object(self, name="", path=None, key=None, namespace=None, 
-                        included_types=None, create="unless_exists",
-                        extra=None, use_permission_of=None, attribution=None):
-        if type(included_types) is str:
-            included_types = [included_types]
-
-        if path and (key or namespace):
-            raise ValueError("You can't specify both the path and a key and namespace.")
-
-        if path:
-            key, namespace = get_key_namespace(path)
-
-        if (key and not namespace) or (not key and namespace):
-            raise ValueError("You must either specify both a key and a namespace, or neither.")
-
-        if included_types:
-            its = set(included_types)
-            q = [{
-                "id|=" : included_types,
-                "/freebase/type_hints/included_types" : [{"id" : None}]
-            }]
-            for res in self.mqlread(q):
-                its.update([x["id"] for x in res["/freebase/type_hints/included_types"]])
-
-        wq = {
-            "id" : None,
-            "name" : name,
-            "create" : create
-        }
-
-        # conditionally add key creation
-        if key:
-            wq.update({"key" : { 
-                "namespace" : namespace,
-                "value" : key,
-            }})
-
-        if included_types:
-            wq.update(type = [{ "id" : it, "connect" : "insert" } for it in its])
-
-        if extra: 
-            wq.update(extra)
-
-        return self.mqlwrite(wq, use_permission_of=use_permission_of, attribution_id=attribution)
-
-
-    def connect_object(self, id, newpath, extra=None, use_permission_of=None, attribution=None):
-
-        key, namespace = get_key_namespace(newpath)
-
-        wq = {
-            "id" : id,
-            "key" : {
-                "namespace" : namespace,
-                "value" : key,
-                "connect" : "insert"
-            }
-        }
-
-        if extra: wq.update(extra)
-
-        return self.mqlwrite(wq, use_permission_of=use_permission_of, attribution_id=attribution)
-
-
-    def disconnect_object(self, id, extra=None, use_permission_of=None, attribution=None):
-
-        key, namespace = get_key_namespace(id)
-
-        wq = {
-            "id" : id,
-            "key" : {
-                "namespace" : namespace,
-                "value" : key,
-                "connect" : "delete"
-            }
-        }
-        if extra: wq.update(extra)
-        return self.mqlwrite(wq, use_permission_of=use_permission_of, attribution_id=attribution)
-
-    def move_object(self, oldpath, newpath, use_permission_of=None, attribution=None):
-        a = self.connect_object(oldpath, newpath, use_permission_of=use_permission_of, attribution=attribution)
-        b = self.disconnect_object(oldpath, use_permission_of=use_permission_of, attribution=attribution)
-        return a, b
-    
-
-
-def get_key_namespace(path):
-    # be careful with /common
-    namespace, key = path.rsplit("/", 1)
-    return (key, namespace or "/")
-
 
 if __name__ == '__main__':
     console = logging.StreamHandler()
diff --git a/linkmerge.py b/freebase/experimental/linkmerge.py
index 3cfe1f1..3cfe1f1 100644
--- a/linkmerge.py
+++ b/freebase/experimental/linkmerge.py
diff --git a/merge.py b/freebase/experimental/merge.py
index 3e9dd04..0d7ddf7 100644
--- a/merge.py
+++ b/freebase/experimental/merge.py
@@ -45,7 +45,7 @@ def merge(s, amoeba_id, target_id):
 
     # type amoeba with new types in target
     for type_id in target_types:
-        freebase.schema.type_object(s, amoeba_id, type_id)
+        freebase.schema.add_type_to_object(s, amoeba_id, type_id)
 
     # get all properties of target
     mega_target_query = { "id" : target_id }
@@ -97,8 +97,19 @@ def merge(s, amoeba_id, target_id):
     s.mqlwrite(master_write_amoeba_query)
     
     # make name of target an alias in amoeba
-    make_alias_query = {"id" : amoeba_id,
-                        "alias" : dddddddddddddddddd}
+    get_amoeba_name_query = {"id" : amoeba_id,
+                             "name" : None }
+    get_target_alias_query = {"id" : target_id,
+                              "/common/topic/alias" : [{}] }
+    amoeba_name, target_aliases = s.mqlread([get_amoeba_name_query,
+                                            get_target_alias_query])
+    # TODO:
+    #set_alias_query = {"id" : amoeba_id}
+    #for alias in target_aliases["/common/topic/alias"]:
+    #    if alias.value != amoeba_name:
+    #        set_alias_query.update({"lang": "/lang/en",
+    #        "value": "Beeetles",
+    #        "connect": "insert"})
     
     # migrate data (thinks linking here)
     # get all the links from the target to someone else
diff --git a/split.py b/freebase/experimental/split.py
index a8d0310..a8d0310 100644
--- a/split.py
+++ b/freebase/experimental/split.py
diff --git a/freebase/fcl/commands.py b/freebase/fcl/commands.py
index 4b517ec..4de2496 100755
--- a/freebase/fcl/commands.py
+++ b/freebase/fcl/commands.py
@@ -34,6 +34,7 @@ import simplejson
 import freebase.rison as rison
 from freebase.api import attrdict
 
+from freebase.schema import connect_object, disconnect_object
 
 def cmd_help(fb, command=None):
     """get help on commands
@@ -187,17 +188,8 @@ def cmd_ln(fb, src, dst):
     """
     src = fb.absid(src)
     dst = fb.absid(dst)
-    dir,file = dirsplit(dst)
-    wq = { 'id': src,
-           'key':{
-               'connect': 'insert',
-               'namespace': dir,
-               'value': file
-               }
-         }
-
-    r = fb.mss.mqlwrite(wq)
-
+    
+    return connect_object(fb.mss, src, dst)
 
 def cmd_rm(fb, path):
     """unlink a namespace key
@@ -213,17 +205,8 @@ def cmd_rm(fb, path):
     disturb anything other than the one directory entry.
     """
     path = fb.absid(path)
-    dir,file = dirsplit(path)
-
-    wq = { 'id': path,
-           'key':{
-               'connect': 'delete',
-               'namespace': dir,
-               'value': file
-            }
-         }
-
-    r = fb.mss.mqlwrite(wq)
+    
+    return disconnect_object(fb.mss, path)
 
 def cmd_mv(fb, src, dst):
     """rename srcid to dstid.
diff --git a/freebase/fcl/fcl.py b/freebase/fcl/fcl.py
index a0299b8..8b484c5 100755
--- a/freebase/fcl/fcl.py
+++ b/freebase/fcl/fcl.py
@@ -254,6 +254,7 @@ class FbCommandHandler(object):
 
         self.import_commands('freebase.fcl.commands')
         self.import_commands('freebase.fcl.mktype')
+        self.import_commands('freebase.fcl.schema')
 
         cmd = args.pop(0)
         self.dispatch(cmd, args)
diff --git a/freebase/fcl/mktype.py b/freebase/fcl/mktype.py
index 30f8df7..f84d017 100755
--- a/freebase/fcl/mktype.py
+++ b/freebase/fcl/mktype.py
@@ -29,6 +29,7 @@
 import os, sys, re, time
 from fbutil import *
 
+from freebase.schema import create_object, create_type
 
 def cmd_mkobj(fb, id, typeid='/common/topic', name=''):
     """create a new object with a given type  -- EXPERIMENTAL
@@ -41,28 +42,10 @@ def cmd_mkobj(fb, id, typeid='/common/topic', name=''):
     
     """
     id = fb.absid(id)
-    nsid, key = dirsplit(id)
-
-    typeid = fb.absid(typeid)
-
-    if name == '':
-        name = key
-
-    wq = { 'create': 'unless_exists',
-           'id': None,
-           'name': name,
-           'type': typeid,
-           'key':{
-               'namespace': nsid,
-               'value': key
-           },
-         }
-
-    # TODO add included types
-
-    r = fb.mss.mqlwrite(wq)
-    print r.id,r.create
-
+    
+    return create_object(fb.mss, name="", path=id, 
+                        included_types=type_id, create="unless_exists")
+                
 def cmd_mktype(fb, id, name=''):
     """create a new type  -- EXPERIMENTAL
     %prog mktype new_id name
@@ -73,26 +56,12 @@ def cmd_mktype(fb, id, name=''):
     this doesn't create any type hints.
 
     if present, name gives the display name of the new property
+    
+    For more options when creating a type, use the python library
     """
     id = fb.absid(id)
-
-    nsid, key = dirsplit(id)
-
-    if name == '':
-        name = key
-
-    wq = { 'create': 'unless_exists',
-           'id': None,
-           'name': name,
-           'type': '/type/type',
-           'key':{
-               'namespace': nsid,
-               'value': key
-           },
-         }
-
-    r = fb.mss.mqlwrite(wq)
-    print r.id,r.create
+    ns, key = dirsplit(id)
+    return create_type(s, name, key, ns, cvt=False, tip=None, included=None, extra=None):
 
 def mkprop(fb, typeid, key, name='', vtype=None, master_property=None):
     """helper to create a new property
diff --git a/freebase/fcl/schema.py b/freebase/fcl/schema.py
new file mode 100644
index 0000000..47eb1fa
--- /dev/null
+++ b/freebase/fcl/schema.py
@@ -0,0 +1,41 @@
+from freebase.schema import dump_base, dump_type, restore
+
+try:
+    import jsonlib2 as json
+except ImportError:
+    try:
+        import simplejson as json
+    except ImportError:
+        import json
+
+import sys
+
+def cmd_dump_base(fb, baseid):
+    """dump a base to stdout
+    %prog dump_base baseid
+
+    Dump a base by outputting a json representation
+    of the types and properties involved.
+    """
+    print >> sys.stdout, json.dumps(dump_base(fb.mss, baseid), indent=2)
+
+def cmd_dump_type(fb, baseid, follow_types=True):
+    """dump a type to stdout
+    %prog dump_type typeid [follow_types=True]
+
+    Dump a type by outputting a json representation
+    of the type and properties involved.
+    """
+    print >> sys.stdout, json.dumps(dump_type(fb.mss, typeid, follow_types), indent=2)
+
+def cmd_restore(fb, newlocation, graphfile):
+    """restore a graph object to the graph
+    %prog restore newlocation graphfile
+
+    Restore a graph object to the newlocation
+    """
+    fh = open(graphfile, "r")
+    graph = json.loads(fh.read())
+    fh.close()
+    return restore(fb.mss, graph, newlocation, ignore_types=None)
+
diff --git a/freebase/schema.py b/freebase/schema.py
index 01d6074..4198993 100644
--- a/freebase/schema.py
+++ b/freebase/schema.py
@@ -1,6 +1,7 @@
+from copy import deepcopy
 
-from freebase.api.session import HTTPMetawebSession
-from freebase.api.session import get_key_namespace, LITERAL_TYPE_IDS
+from freebase.api.session import HTTPMetawebSession, MetawebError
+from freebase.api.session import LITERAL_TYPE_IDS
 
 """
 NOTE
@@ -8,7 +9,8 @@ NOTE
 graph is used freely in this file. Some information:
  - It refers to an internal representation of a group of types.
  - It resembles a mqlread result, but it is not a mqlread result
- - It also has some helper variables like __requires and __related. 
+ - It also has some helper variables like __requires and __related.
+ - All helper variables start with __ since that's not valid MQL
  - It is produced by _get_graph
  - It can be converted into valid json (json.dumps(graph, indent=2))
  
@@ -19,7 +21,7 @@ Its structure is as follows:
       "id" : "/base_id/type_id"
       ...
       "__requires" : ["/base_id/type_id2"]
-      "__properties" : [
+      "properties" : [
         {
           "id" : "/base_id/type_id/my_prop"
           ...
@@ -47,8 +49,119 @@ def key_exists(s, k):
     }
     return not None == s.mqlread(q)
 
+### SCHEMA MANIPULATION ###
+# Object helpers
+def create_object(s, name="", path=None, key=None, namespace=None, 
+                    included_types=None, create="unless_exists",
+                    extra=None, use_permission_of=None, attribution=None):
+    """
+    Create object with name, a path or a key and namespace, included_types.
+    You can also specify how it is created (unless_exists, unconditional..)
+    as well as use_permission_of and attribution
+    """
+    if type(included_types) is str:
+        included_types = [included_types]
 
-def type_object(s, id, type_id):
+    if path and (key or namespace):
+        raise ValueError("You can't specify both the path and a key and namespace.")
+
+    if path:
+        key, namespace = get_key_namespace(path)
+
+    if (key and not namespace) or (not key and namespace):
+        raise ValueError("You must either specify both a key and a namespace, or neither.")
+
+    if included_types:
+        its = set(included_types)
+        q = [{
+            "id|=" : included_types,
+            "/freebase/type_hints/included_types" : [{"id" : None}]
+        }]
+        for res in s.mqlread(q):
+            its.update([x["id"] for x in res["/freebase/type_hints/included_types"]])
+
+    wq = {
+        "id" : None,
+        "name" : name,
+        "create" : create
+    }
+
+    # conditionally add key creation
+    if key:
+        wq.update({"key" : { 
+            "namespace" : namespace,
+            "value" : key,
+        }})
+
+    if included_types:
+        wq.update(type = [{ "id" : it, "connect" : "insert" } for it in its])
+
+    if extra: 
+        wq.update(extra)
+
+    return s.mqlwrite(wq, use_permission_of=use_permission_of, attribution_id=attribution)
+
+
+def connect_object(s, id, newpath, extra=None, use_permission_of=None, attribution=None):
+    """ connect object at id to a newpath.
+    Example:
+    connect_object(s, "/guid/002", "/en/the_beatles")
+    """
+    key, namespace = get_key_namespace(newpath)
+
+    wq = {
+        "id" : id,
+        "key" : {
+            "namespace" : namespace,
+            "value" : key,
+            "connect" : "insert"
+        }
+    }
+
+    if extra: wq.update(extra)
+
+    return s.mqlwrite(wq, use_permission_of=use_permission_of, attribution_id=attribution)
+
+
+def disconnect_object(s, id, extra=None, use_permission_of=None, attribution=None):
+    """ disconnect objects, as in removing their keys
+    Example:
+    disconnect_object(s, "/en/the_beatles")
+    would remove the `the_beatles` key from /en """
+    key, namespace = get_key_namespace(id)
+
+    wq = {
+        "id" : id,
+        "key" : {
+            "namespace" : namespace,
+            "value" : key,
+            "connect" : "delete"
+        }
+    }
+    if extra: wq.update(extra)
+    return s.mqlwrite(wq, use_permission_of=use_permission_of, attribution_id=attribution)
+
+def move_object(s, oldpath, newpath, use_permission_of=None, attribution=None):
+    """ move object from one key to another."""
+    a = connect_object(s, oldpath, newpath, use_permission_of=use_permission_of, attribution=attribution)
+    b = disconnect_object(s, oldpath, use_permission_of=use_permission_of, attribution=attribution)
+    return a, b
+
+
+def get_key_namespace(path):
+    """ get (key, namespace) from a path
+    get_key_namespace("/common") -> ("common", "/")
+    get_key_namespace("/food/drinks") -> ("drinks", "/food")
+    """
+    # be careful with /common
+    namespace, key = path.rsplit("/", 1)
+    return (key, namespace or "/")
+
+
+def add_type_to_object(s, id, type_id):
+    """
+     given an object (id) give it the type, type_id and all of its included types.
+    """
     q = {
         "id" : type_id,
         "/freebase/type_hints/included_types" : [{"id" : None, "optional" : True}]
@@ -66,21 +179,39 @@ def type_object(s, id, type_id):
 
 
 def copy_property(s, id, newid, **extra):
+    """ create a new property with the same information as the starting property """
     newname, newschema = get_key_namespace(newid)
     
     info = get_property_info(s, id)
     info["__raw"].update(extra)
-    create_property(s, info["name"], newname, newschema, info["expected_type"], info["unique"], info["/freebase/property_hints/disambiguator"],
-        info["/freebase/documented_object/tip"], info["__raw"])
+    
+    unique = None
+    if info.has_key(unique):
+        unique = info["unique"]
+    disambig = None
+    if info.has_key("/freebase/property_hints/disambiguator"):
+        disambig = info["/freebase/property_hints/disambiguator"]
+    create_property(s, info["name"], newname, newschema, info["expected_type"], unique=unique, disambig=disambig,
+        tip=info["/freebase/documented_object/tip"], extra=info["__raw"])
 
 def move_property(s, id, newid, **extra):
+    """ create an identical property and delete the old one """
     copy_property(s, id, newid, **extra)
-    disconnect_schema = {"type" : "/type/property", "schema" : {"connect" : "delete", "id" : "/".join(id.split("/")[:-1]) }}    
-    s.disconnect_object(id, extra = disconnect_schema)
+    disconnect_schema = {"type" : "/type/property", "schema" : {"connect" : "delete", "id" : "/".join(id.split("/")[:-1]) }}  
+    disconnect_object(s, id, extra = disconnect_schema)
 
 def get_property_info(s, prop_id):
-    q = PROPERTY_QUERY
+    """
+    get_property_info returns a valid json dictionary object that has all the information
+    required to describe a property. This is only used by copy_property, but could be used
+    by whoever.
+    
+    Ideally, all the required information by create_property is in the root of the dictionary
+    while all the extra information is in result["__raw"]
+    """
+    q = deepcopy(PROPERTY_QUERY)
     q.update(id=prop_id)
+    q.update(schema={"id" : None, "name" : None})
     res = s.mqlread(q)
     info = {}
     
@@ -88,33 +219,23 @@ def get_property_info(s, prop_id):
     if res["schema"]:
         info["schema"] = res["schema"]["id"]
     else: info["schema"] = None
-
+    
     if res["key"]:
-        info["keys"] = map(lambda x: (x["value"], x["namespace"]), res["key"])
+        info["key"] = [(x["value"], x["namespace"]) for x in res["key"]]
     else: info["key"] = None
 
     if res["/freebase/documented_object/tip"]:
         info["/freebase/documented_object/tip"] = res["/freebase/documented_object/tip"]["value"]
     else: info["/freebase/documented_object/tip"] = None
+    
+    ignore = ("optional", "type", "key", "/freebase/documented_object/tip")
+    for prop in PROPERTY_QUERY.iterkeys():
+        if prop not in ignore:
+            if not info.has_key(prop):
+                info[prop] = None
 
-    for i in ["delegated", "enumeration", "expected_type", "id", "master_property", "unique", "unit",
-     "/freebase/property_hints/disambiguator", "/freebase/property_hints/display_none",
-     "/freebase/property_hints/display_orientation","/freebase/property_hints/enumeration",
-      "/freebase/property_hints/dont_display_in_weblinks", "/freebase/property_hints/inverse_description"]:
-
-        if res[i]:
-            if isinstance(res[i], basestring):
-                info[i] = res[i]
-            elif isinstance(res[i], bool):
-                info[i] = res[i]
-            elif res[i].has_key("id"):
-                info[i] = res[i]["id"]
-            elif res[i].has_key("value"):
-                info[i] = res[i]["value"]
-            else:
-                raise ValueError("There is a problem with getting the property value.")
-        else: info[i] = None
-
+    info.update(_generate_extra_properties(res, ignore))
+    
     # delete the properties that are going to be asked for in create_property
     del res["name"]
     del res["schema"]
@@ -123,7 +244,7 @@ def get_property_info(s, prop_id):
     del res["unique"]
     del res["/freebase/property_hints/disambiguator"]
     del res["/freebase/documented_object/tip"]
-
+    
     # delete other useless things...
     del res["id"]
 
@@ -136,6 +257,10 @@ def get_property_info(s, prop_id):
 
 # Create Type
 def create_type(s, name, key, ns, cvt=False, tip=None, included=None, extra=None):
+    """
+    creates a type and takes care of associating it to its domain and attaching
+    a key.
+    """
     # TODO: CREATE SYNTHETIC VIEW
     if key_exists(s, ns + "/" + key ):
         return
@@ -148,7 +273,7 @@ def create_type(s, name, key, ns, cvt=False, tip=None, included=None, extra=None
     assert included is None or isinstance(included, (basestring, list, tuple))
     assert extra is None or isinstance(extra, dict)
 
-    q = {
+    wq = {
         "create" : "unconditional",
         "type" : "/type/type",
         "/type/type/domain" : { "connect" : "insert", "id" : ns },
@@ -163,36 +288,38 @@ def create_type(s, name, key, ns, cvt=False, tip=None, included=None, extra=None
     if included:
         if isinstance(included, basestring):
             included = [included]
-        itsq = [{
+        its_q = [{
             "id|=" : included,
             "/freebase/type_hints/included_types" : [{"id" : None}]
         }]
-        r = s.mqlread(itsq)
+        r = s.mqlread(its_q)
         included_types = set(included)
         if r:
             for i in r:
                 included_types.update(map(lambda x: x["id"], i["/freebase/type_hints/included_types"]))
 
         its = [{"connect" : "insert", "id" : t} for t in included_types]
-        q['/freebase/type_hints/included_types'] = its
+        wq['/freebase/type_hints/included_types'] = its
 
     # TODO: enum
 
     if cvt:
-        q['/freebase/type_hints/mediator'] = { "connect" : "update", "value" : True }
+        wq['/freebase/type_hints/mediator'] = { "connect" : "update", "value" : True }
     if tip:
-        q['/freebase/documented_object/tip'] = { "connect" : "update", "value" : tip, "lang" : "/lang/en" }
+        wq['/freebase/documented_object/tip'] = { "connect" : "update", "value" : tip, "lang" : "/lang/en" }
     
-    if extra: q.update(extra)
-    return s.mqlwrite(q, use_permission_of=ns)
+    if extra: wq.update(extra)
+    return s.mqlwrite(wq, use_permission_of=ns)
 
 
 # Create Property
 def create_property(s, name, key, schema, expected, unique=False, disambig=False, tip=None, extra=None):
+    """
+    create a property with ect, unique, etc, and make schema and key links
+    """
     if key_exists(s, schema + "/" + key):
         return
 
-    
     # validate parameters
     # assert isinstance(name, basestring) # could be mql
     assert isinstance(key, basestring)
@@ -201,7 +328,7 @@ def create_property(s, name, key, schema, expected, unique=False, disambig=False
     assert tip is None or isinstance(tip, basestring)    
     assert extra is None or isinstance(extra, dict)
     
-    q = {
+    wq = {
         "create" : "unconditional",
         "type" : "/type/property",
         "name" : name,
@@ -214,20 +341,21 @@ def create_property(s, name, key, schema, expected, unique=False, disambig=False
         "expected_type" : { "connect" : "insert", "id" : expected }
     }
     if unique:
-         q['unique'] = { "connect" : "update", "value" : unique }
+         wq['unique'] = { "connect" : "update", "value" : unique }
     if tip:
-         q['/freebase/documented_object/tip'] = { "connect" : "update", "value" : tip, "lang" : "/lang/en" }
+         wq['/freebase/documented_object/tip'] = { "connect" : "update", "value" : tip, "lang" : "/lang/en" }
     if disambig:
-         q['/freebase/property_hints/disambiguator'] = { "connect" : "update", "value" : True }
+         wq['/freebase/property_hints/disambiguator'] = { "connect" : "update", "value" : True }
     if extra:
-         q.update(extra)
-    return s.mqlwrite(q, use_permission_of=schema)
+         wq.update(extra)
+    return s.mqlwrite(wq, use_permission_of=schema)
 
 def delegate_property(s, p, schema, name=None, key=None, expected=None, tip=None, extra=None):
-    
+    """
+    create a property with a delegate
+    """
     assert isinstance(p, basestring)
     assert isinstance(schema, basestring)
-    #assert name is None or isinstance(name, basestring)
     assert key is None or isinstance(key, basestring)
     assert expected is None or isinstance(expected, basestring)
     assert tip is None or isinstance(tip, basestring)
@@ -299,15 +427,23 @@ def reciprocate_property(s, name, key, master, unique=False, disambig=False, tip
     assert tip is None or isinstance(tip, basestring)
     assert extra is None or isinstance(extra, dict)
     
-
+    # get master information
     q = {
         "id" : master,
         "/type/property/expected_type" : None,
-        "/type/property/schema" : None }
+        "/type/property/schema" : None,
+        "/type/property/reverse_property" : None }
     r = s.mqlread(q)
     ect = r["/type/property/expected_type"]
     schema = r["/type/property/schema"]
 
+    # check to see if a master existed
+    if r["/type/property/reverse_property"]:
+        raise MetawebError("You can't reciprocate property %s who \
+                            already has a reverse property %s",
+                            (master, r["/type/property/reverse_property"]))
+
+
     master = {"master_property" : master}
     if extra: master.update(extra)
 
@@ -317,20 +453,30 @@ def reciprocate_property(s, name, key, master, unique=False, disambig=False, tip
 
 # dump / restore types
 def dump_base(s, base_id):
-    types = [type_object["id"] for type_object in s.mqlread({"id" : base_id, "/type/domain/types":[{"id" : None}]})["/type/domain/types"]]
+    """ dump a base into a `graph` object. See information at the top of the file for more
+    information on the graph file """
+    types = [add_type_to_object["id"] for type_object in s.mqlread({"id" : base_id, "/type/domain/types":[{"id" : None}]})["/type/domain/types"]]
     graph = _get_graph(s, types, True)
         
     return graph
 
 def dump_type(s, type_id, follow_types=True):
+    """ dump a type (similar to dump_base) and has an argument follow_types that determines
+    if it should dump types that are neccessary or just rely on them """
     types = [type_id]
     graph = _get_graph(s, types, follow_types)
     
     return graph
 
 def restore(s, graph, new_location, ignore_types=None):
+    """ given a `graph` object and a another location, we can upload our graph output
+    into a new location in the freebase graph """
     follow_types = graph.get("__follow_types", True)
     
+    # We assume the new_location is empty. if it isn't, we bail.
+    # well, ...
+    
+    
     # create type dependencies
     type_requires_graph = {}
     
@@ -376,8 +522,7 @@ def restore(s, graph, new_location, ignore_types=None):
         if graph[type_id]["/freebase/documented_object/tip"]:
             tip = graph[type_id]["/freebase/documented_object/tip"]["value"]
         
-        ignore = ("name", "domain", "key", "type", "id", "properties", "/freebase/type_hints/enumeration",
-                    "/freebase/type_hints/included_types", "/freebase/type_hints/mediator", "/freebase/documented_object/tip")
+        ignore = TYPE_INGORE_PROPERTIES
         extra = _generate_extra_properties(graph[type_id], ignore)
         
         name = graph[type_id]["name"]["value"]
@@ -386,7 +531,7 @@ def restore(s, graph, new_location, ignore_types=None):
         
         create_type(s, name, key, new_location_id, included=included, cvt=cvt, tip=tip, extra=extra)
     
-    for prop_id in props_to_create: #* prop_id
+    for prop_id in props_to_create:
         type_id = prop_to_type_map[prop_id]
         all_properties_for_type = graph[type_id]["properties"]
         for prop in all_properties_for_type:
@@ -412,17 +557,14 @@ def restore(s, graph, new_location, ignore_types=None):
                 disambig = prop["/freebase/property_hints/disambiguator"]
                 unique = prop["unique"]
                 
-                ignore = ("name", "expected_type", "key", "id", "master_property", "delegated", "unique", "type", "schema",
-                            "/freebase/property_hints/disambiguator", "enumeration", "/freebase/property_hints/enumeration", 
-                            "/freebase/documented_object/tip")
-                
+                ignore = PROPERTY_IGNORE_PROPERTIES
                 extra = _generate_extra_properties(prop, ignore)   
                 
                 if prop['master_property']:
                     converted_master_property = _convert_name_to_new(prop["master_property"], origin_id, new_location_id, only_include)
                     if converted_master_property == prop["master_property"]:
                         raise CVTError("You can't set follow_types to False if there's a cvt. A cvt requires you get all the relevant types. Set follow_types to true.\n" + \
-                                        "The offending property was %s, whose master was %s." % (prop["id"], prop["master_property"]))
+                                       "The offending property was %s, whose master was %s." % (prop["id"], prop["master_property"]))
                     reciprocate_property(s, name, key, converted_master_property,
                         unique, disambig=disambig, tip=tip, extra=extra)
                 
@@ -431,9 +573,8 @@ def restore(s, graph, new_location, ignore_types=None):
                         expected=expected, tip=tip, extra=extra)
                 
                 else:
-                    create_property(s, name, key, new_schema, expected, unique, 
-                        disambig=disambig, tip=tip, extra=extra) 
-                        
+                    create_property(s, name, key, new_schema, expected, unique,
+                        disambig=disambig, tip=tip, extra=extra)
 
 def _get_graph(s, initial_types, follow_types):
     """ get the graph of dependencies of all the types involved, starting with a list supplied """
@@ -517,10 +658,13 @@ def _generate_extra_properties(dictionary_of_values, ignore):
                     extra.update({k:v["value"]})
                 else:
                     raise ValueError("There is a problem with getting the property value.")
+            else:
+                if isinstance(v, bool): # well, if its False...
+                    extra.update({k:v})
     return extra
 
 def _get_needed(s, type_id):
-    q = TYPE_QUERY
+    q = deepcopy(TYPE_QUERY)
     q.update(id=type_id)
 
     r = s.mqlread(q)
@@ -557,7 +701,7 @@ def _get_needed(s, type_id):
 
     # return all the information along with our special __* properties
     info = r
-    info.update(__related=related, __requires=requires, __properties=properties)
+    info.update(__related=related, __requires=requires)
 
     return info
 
@@ -574,8 +718,6 @@ def _return_relevant(start_list, parents):
             final.append(item)
     return final
 
-
-
 PROPERTY_QUERY = {
         "optional" : True,
         "type" : "/type/property",
@@ -587,10 +729,8 @@ PROPERTY_QUERY = {
             "namespace" : None, 
             "value" : None
         }],
-        #"link" : [{}], 
         "master_property" : None,
         "name" : {"value" : None, "lang" : "/lang/en", "optional":True}, 
-        "schema" : {"id" : None, "name" : None},
         "unique" : None, 
         "unit" : None,
         "/freebase/documented_object/tip" : {"value" : None, "limit":1, "optional" : True},
@@ -613,5 +753,11 @@ TYPE_QUERY = {
         "/freebase/type_hints/minor" : None,
         "/freebase/documented_object/tip" : {"value" : None, "limit":1, "optional":True},
         }
-TYPE_QUERY.update(properties=[PROPERTY_QUERY])    
+TYPE_QUERY.update(properties=[deepcopy(PROPERTY_QUERY)])    
+
+TYPE_INGORE_PROPERTIES = ("name", "domain", "key", "type", "id", "properties", "/freebase/type_hints/enumeration",
+            "/freebase/type_hints/included_types", "/freebase/type_hints/mediator", "/freebase/documented_object/tip")
 
+PROPERTY_IGNORE_PROPERTIES = ("name", "expected_type", "key", "id", "master_property", "delegated", "unique", "type", "schema",
+            "/freebase/property_hints/disambiguator", "enumeration", "/freebase/property_hints/enumeration", 
+            "/freebase/documented_object/tip")
diff --git a/freebase/schema_cmd.py b/freebase/schema_cmd.py
deleted file mode 100644
index 33ae7ae..0000000
--- a/freebase/schema_cmd.py
+++ /dev/null
@@ -1,162 +0,0 @@
-from optparse import OptionParser
-import getpass
-import sys
-
-from freebase.api import HTTPMetawebSession
-
-from freebase.schema import dump_base, dump_type, restore
-
-try:
-    import jsonlib2 as json
-except ImportError:
-    try:
-        import simplejson as json
-    except ImportError:
-        import json
-
-def fb_save_base():
-    op = OptionParser(usage='%prog [options] baseid')
-    
-    op.disable_interspersed_args()
-    
-    op.add_option('-s', '--service', dest='service_host',
-                    metavar='HOST',
-                    default="freebase.com",
-                    help='Freebase HTTP service address:port')
-    
-    op.add_option('-S', '--sandbox', dest='use_sandbox',
-                    default=False, action='store_true',
-                    help='shortcut for --service=sandbox-freebase.com')
-
-    options, args = op.parse_args()
-    
-    service_host = options.service_host
-    if options.use_sandbox:
-        service_host = "sandbox-freebase.com"
-
-    if len(args) < 1:
-        op.error('Required baseid missing')
-    
-    if len(args) > 1:
-        op.error('Too many arguments')
-    
-    s = HTTPMetawebSession(service_host)
-    
-    print >> sys.stdout, json.dumps(dump_base(s, args[0]), indent=2)
-    
-def fb_save_type():
-    op = OptionParser(usage='%prog [options] typeid ')
-    
-    op.disable_interspersed_args()
-
-    op.add_option('-s', '--service', dest='service_host',
-                    metavar='HOST',
-                    default="freebase.com",
-                    help='Freebase HTTP service address:port')
-    
-    op.add_option('-S', '--sandbox', dest='use_sandbox',
-                    default=False, action='store_true',
-                    help='shortcut for --service=sandbox-freebase.com')
-    
-    op.add_option('-n', '--no-follow', dest='follow',
-                    default=False, action='store_false',
-                    help="Don't follow types, only copy the one specified.")
-    
-    op.add_option('-f', '--follow', dest="follow",
-                    default=True, action="store_true",
-                    help="Follow the types (you might end up copying multiple types)")
-
-
-    options,args = op.parse_args()
-    
-    service_host = options.service_host
-    if options.use_sandbox:
-        service_host = "sandbox-freebase.com"
-
-    if len(args) < 1:
-        op.error('Required typeid missing')
-    
-    if len(args) > 1:
-        op.error('Too many arguments')
-    
-    s = HTTPMetawebSession(service_host)
-    print >> sys.stdout, json.dumps(dump_type(s, args[0], options.follow), indent=2)
-    
-
-def fb_restore():
-    op = OptionParser(usage='%prog [options] new_location graph_output_from_dump*_command')
-    
-    op.disable_interspersed_args()
-
-    op.add_option('-s', '--service', dest='service_host',
-                    metavar='HOST',
-                    help='Freebase HTTP service address:port')
-    
-    op.add_option('-S', '--sandbox', dest='use_sandbox',
-                    default=False, action='store_true',
-                    help='shortcut for --service=sandbox-freebase.com (default)')
-    
-    op.add_option('-F', '--freebase', dest='use_freebase',
-                    default=False, action='store_true',
-                    help='shortcut for --service=freebase.com (not default)')
-
-    op.add_option('-u', '--username', dest='username',
-                    action='store',
-                    help='username for freebase service')
-
-    op.add_option('-p', '--password', dest='password',
-                    action='store',
-                    help='password for freebase service')
-
-                    
-
-    options,args = op.parse_args()
-
-    if (options.username and not options.password) or (not options.username and options.password):
-        op.error("You must supply both a username and password")
-
-    if options.use_sandbox and options.use_freebase:
-        op.error("You can't use both freebase and sandbox!")
-    
-    if options.service_host and (options.use_sandbox or options.use_freebase):
-        op.error("You can't specify both --service and --freebase or --sandbox")
-    
-    if not options.service_host and not options.use_sandbox and not options.use_freebase:
-        op.error("You have to specify to upload to sandbox or production (freebase)")
-    
-    service_host = options.service_host
-    if options.use_sandbox:
-        service_host = "sandbox-freebase.com"
-    if options.use_freebase:
-        service_host = "freebase.com"
-
-    s = login(service_host, username=options.username, password=options.password)
-    
-    newlocation = args[0]
-    if len(args) == 1:
-        graphfile = "-" #stdin
-    else: graphfile = args[1]
-    if graphfile != "-":
-        fg = open(graphfile, "r")
-        graph = json.load(fg)
-        fg.close()
-    if graphfile == "-": # use stdin
-        graph = json.load(sys.stdin)
-        
-    restore(s, graph, newlocation, ignore_types=None)
-
-def login(api_host, username=None, password=None):
-    
-    s = HTTPMetawebSession(api_host)
-    if not username:
-        print "In order to perform this operation, we need to use a valid freebase username and password"
-        username = raw_input("Please enter your username: ")
-        try:
-            password = getpass.getpass("Please enter your password: ")
-        except getpass.GetPassWarning:
-            password = raw_input("Please enter your password: ")
-
-    s.login(username, password)
-
-    print "Thanks!"
-    return s
\ No newline at end of file
diff --git a/test/test_hardcore_schema_manipulation.py b/test/test_hardcore_schema_manipulation.py
index aaecf28..e078a4d 100644
--- a/test/test_hardcore_schema_manipulation.py
+++ b/test/test_hardcore_schema_manipulation.py
@@ -69,9 +69,15 @@ class TestHardcoreSchemaManipulation(unittest.TestCase):
         self.assertEqual([ignore_base(prop_id) for prop_id in realproperties], [ignore_base(prop_id) for prop_id in newproperties])
         
         # - check the properties and type's attributes are the same
+        # TODO
         
+    def test_restore_over_restore(self):
+        domain_id = _create_domain()
+        graph = dump_domain(s, "/base/contractbridge")   
+        restore(s, graph, domain_id)
         
-        
+        # now we restore again... it should raise a MetawebError
+        self.assertRaises(MetawebError, restore(s, graph, domain_id))
     
     def test_try_copying_a_cvt(self):
         
diff --git a/test/test_schema_manipulation.py b/test/test_schema_manipulation.py
index f8453e7..12151d7 100644
--- a/test/test_schema_manipulation.py
+++ b/test/test_schema_manipulation.py
@@ -8,7 +8,9 @@ import getlogindetails
 
 from freebase.api import HTTPMetawebSession, MetawebError
 from freebase.schema import create_type, reciprocate_property, delegate_property
-from freebase.schema import create_property, type_object, copy_property, move_property
+from freebase.schema import create_property, add_type_to_object, copy_property, move_property
+from freebase.schema import create_object, connect_object, disconnect_object
+from freebase.schema import move_object
 
 USERNAME = 'username'
 PASSWORD = 'password'
@@ -31,28 +33,27 @@ f = lambda x: x["id"]
 class TestSchemaManipulation(unittest.TestCase):
     
     def test_make_and_type_object(self):
-        a = s.create_object("A", path=domain_id + "/a")
+        a = create_object(s, "A", path=domain_id + "/a")
         self.assertEqual(a.create, "created")
         
-        b = s.create_object("B", path=domain_id + "/b", included_types=["/people/person"])
+        b = create_object(s, "B", path=domain_id + "/b", included_types=["/people/person"])
         
         q = { "id" : b.id, "type" : [{"id" : None}] }
-        types = map(f, s.mqlread(q)["type"])
+        types = [x["id"] for x in s.mqlread(q)["type"]]
         self.assertEqual("/common/topic" in types, True)
         self.assertEqual("/people/person" in types, True)
         self.assertEqual("/film/actor" in types, False)
         
-        type_object(s, b.id, "/film/film_genre")
+        add_type_to_object(s, b.id, "/film/film_genre")
         
-        types = map(f, s.mqlread(q)["type"])
+        types = [x["id"] for x in s.mqlread(q)["type"]]
         
         self.assertEqual("/film/film_genre" in types, True)
         self.assertEqual("/media_common/media_genre" in types, True)
 
     def test_move_object(self):
-        print domain_id
-        old = s.create_object("old", domain_id + "/old")
-        s.move_object(domain_id + "/old", domain_id + "/new")
+        old = create_object(s, "old", domain_id + "/old")
+        move_object(s, domain_id + "/old", domain_id + "/new")
         is_old = { "id" : domain_id + "/old", "key" : [{"value" : None}]}
         is_new = { "id" : domain_id + "/new", "key" : [{"value" : None}]}
         s.touch()
@@ -130,6 +131,27 @@ class TestSchemaManipulation(unittest.TestCase):
         #delegator property test
         delegate_property(s, "/people/person/date_of_birth", player, "Date of Birth", "db")
 
+    def test_create_over_created(self):
+        """ No creating a property on top of an already created one..."""
+        create_type(s, "Rapper", "rapper", domain_id)
+        create_property(s, "Styles", "styles", domain_id+"/rapper", "/music/genre")
+        
+        # now we mistakenly create the same prop
+        # it should just exit as if it finished successfully... but, it shouldn't actually do anything
+        create_property(s, "Styles", "styles", domain_id+"/rapper", "/music/genre")
+        # we also mistakenly create the same type, again
+        create_type(s, "Rapper", "rapper", domain_id)
+    
+    def test_reciprocating_reciprocated(self):
+        """ You can't reciprocate an already reciprocated property"""
+        create_type(s, "Master", "master", domain_id)
+        create_type(s, "Servant", "servant", domain_id)
+        
+        create_property(s, "Servants", "servants", domain_id + "/master", domain_id + "/servant", unique=False)
+        reciprocate_property(s, "Masters", "masters", domain_id + "/master/servants", unique=False)
+        
+        self.assertRaises(MetawebError, lambda: reciprocate_property(s, "Buddies", "buddies", domain_id + "/master/servants", unique=False))
+
 if __name__ == '__main__':
     unittest.main()