Resolve code smell issues

Author: michael-koeller

docx/opc/customprops.py

@@ -13,6 +13,7 @@
 
 NS_VT = "http://schemas.openxmlformats.org/officeDocument/2006/docPropsVTypes"
 
+
 class CustomProperties(object):
     """
     Corresponds to part named ``/docProps/custom.xml``, containing the custom
@@ -21,50 +22,51 @@ class CustomProperties(object):
     def __init__(self, element):
         self._element = element
 
-    def __getitem__( self, item ):
+    def __getitem__(self, item):
         prop = self.lookup(item)
-        if prop is not None :
-            # print(etree.tostring(prop, pretty_print=True))
+        if prop is not None:
             elm = prop[0]
             if elm.tag == '{%s}i4' % NS_VT:
                 try:
                     return int(elm.text)
-                except:
+                except ValueError:
                     return elm.text
             elif elm.tag == '{%s}bool' % NS_VT:
                 return True if elm.text == '1' else False
             return elm.text
 
-    def __setitem__( self, key, value ):
+    def __setitem__(self, key, value):
         prop = self.lookup(key)
-        if prop is None :
-            elmType = 'lpwstr'
+        if prop is None:
+            elm_type = 'lpwstr'
             if isinstance(value, bool):
-                elmType = 'bool'
+                elm_type = 'bool'
                 value = str(1 if value else 0)
             elif isinstance(value, numbers.Number):
-                elmType = 'i4'
+                elm_type = 'i4'
                 value = str(int(value))
-            prop = etree.SubElement( self._element, "property" )
-            elm = etree.SubElement(prop, '{%s}%s' %(NS_VT, elmType), nsmap = {'vt':NS_VT} )
+            prop = etree.SubElement(self._element, "property")
+            elm = etree.SubElement(prop, '{%s}%s' % (NS_VT, elm_type), nsmap={'vt': NS_VT})
             elm.text = value
             prop.set("name", key)
+            # magic number "FMTID_UserDefinedProperties"
+            # MS doc ref: https://learn.microsoft.com/de-de/windows/win32/stg/predefined-property-set-format-identifiers
             prop.set("fmtid", "{D5CDD505-2E9C-101B-9397-08002B2CF9AE}")
-            prop.set("pid", "%s" % str(len(self._element) + 1))
+            prop.set("pid", str(len(self._element) + 1))
         else:
             elm = prop[0]
             if elm.tag == '{%s}i4' % NS_VT:
                 elm.text = str(int(value))
             elif elm.tag == '{%s}bool' % NS_VT:
                 elm.text = str(1 if value else 0)
             else:
-                elm.text = '%s' % str(value)
+                elm.text = str(value)
 
-    def __len__( self ):
+    def __len__(self):
         return len(self._element)
 
     def lookup(self, item):
-        for child in self._element :
-            if child.get("name") == item :
+        for child in self._element:
+            if child.get("name") == item:
                 return child
-        return None
+        return None

docx/opc/parts/customprops.py

@@ -10,13 +10,11 @@
 
 from lxml import etree
 
-from datetime import datetime
-
-from ..constants import CONTENT_TYPE as CT
-from ..customprops import CustomProperties
-from ...oxml.customprops import CT_CustomProperties
-from ..packuri import PackURI
-from ..part import XmlPart
+from docx.opc.constants import CONTENT_TYPE as CT
+from docx.opc.customprops import CustomProperties
+from docx.oxml.customprops import CT_CustomProperties
+from docx.opc.packuri import PackURI
+from docx.opc.part import XmlPart
 
 # configure XML parser
 parser_lookup = etree.ElementDefaultClassLookup(element=CT_CustomProperties)
@@ -35,7 +33,6 @@ def ct_parse_xml(xml):
     return root_element
 
 
-
 class CustomPropertiesPart(XmlPart):
     """
     Corresponds to part named ``/docProps/custom.xml``, containing the custom

docx/oxml/__init__.py

@@ -42,8 +42,9 @@ def register_element_cls(tag, cls):
 def register_element_cls_ns(tag, ns, cls):
     """
     Register *cls* to be constructed when the oxml parser encounters an
-    element with matching *tag*. *tag* is a string of the form
-    ``nspfx:tagroot``, e.g. ``'w:document'``.
+    element with matching *tag*.
+    *tag* is a string of the form ``tagroot``, e.g. ``'Properties'``,
+    while the namespace is given explicitely by the *ns* argument.
     """
     namespace = element_class_lookup.get_namespace(ns)
     namespace[tag] = cls

docx/oxml/customprops.py

@@ -8,13 +8,13 @@
     absolute_import, division, print_function, unicode_literals
 )
 
+from datetime import datetime, timedelta
 import re
 
-from datetime import datetime, timedelta
-from lxml import etree
-from .ns import nsdecls, qn
-from .xmlchemy import BaseOxmlElement, ZeroOrOne
-from . import parse_xml
+from docx.oxml.ns import nsdecls, qn
+from docx.oxml.xmlchemy import BaseOxmlElement
+from docx.oxml import parse_xml
+
 
 class CT_CustomProperties(BaseOxmlElement):
     """
@@ -24,17 +24,18 @@ class CT_CustomProperties(BaseOxmlElement):
     """
 
     _customProperties_tmpl = (
-        '<Properties xmlns="http://schemas.openxmlformats.org/officeDocument/2006/custom-properties" %s/>\n' % nsdecls('vt')
+            '<Properties xmlns="http://schemas.openxmlformats.org/officeDocument/2006/custom-properties" %s/>\n' % nsdecls('vt')
     )
+    _offset_pattern = re.compile("([+-])(\\d\\d):(\\d\\d)")
 
     @classmethod
     def new(cls):
         """
         Return a new ``<property>`` element
         """
         xml = cls._customProperties_tmpl
-        customProperties = parse_xml(xml)
-        return customProperties
+        custom_properties = parse_xml(xml)
+        return custom_properties
 
     def _datetime_of_element(self, property_name):
         element = getattr(self, property_name)
@@ -75,8 +76,6 @@ def _offset_dt(cls, dt, offset_str):
         td = timedelta(hours=hours, minutes=minutes)
         return dt + td
 
-    _offset_pattern = re.compile('([+-])(\d\d):(\d\d)')
-
     @classmethod
     def _parse_W3CDTF_to_datetime(cls, w3cdtf_str):
         # valid W3CDTF date cases:
@@ -102,8 +101,7 @@ def _parse_W3CDTF_to_datetime(cls, w3cdtf_str):
             except ValueError:
                 continue
         if dt is None:
-            tmpl = "could not parse W3CDTF datetime string '%s'"
-            raise ValueError(tmpl % w3cdtf_str)
+            raise ValueError("could not parse W3CDTF datetime string '%s'" % {w3cdtf_str})
         if len(offset_str) == 6:
             return cls._offset_dt(dt, offset_str)
         return dt
@@ -113,18 +111,15 @@ def _set_element_datetime(self, prop_name, value):
         Set date/time value of child element having *prop_name* to *value*.
         """
         if not isinstance(value, datetime):
-            tmpl = (
-                "property requires <type 'datetime.datetime'> object, got %s"
-            )
-            raise ValueError(tmpl % type(value))
+            raise ValueError("property requires <type 'datetime.datetime'> object, got %s" % type(value))
         element = self._get_or_add(prop_name)
         dt_str = value.strftime('%Y-%m-%dT%H:%M:%SZ')
         element.text = dt_str
         if prop_name in ('created', 'modified'):
-            # These two require an explicit 'xsi:type="dcterms:W3CDTF"'
-            # attribute. The first and last line are a hack required to add
+            # These two require an explicit 'xsi:type="dcterms:W3CDTF"' attribute.
+            # The first and last line are a hack required to add
             # the xsi namespace to the root element rather than each child
-            # element in which it is referenced
+            # element in which it is referenced.
             self.set(qn('xsi:foo'), 'bar')
             element.set(qn('xsi:type'), 'dcterms:W3CDTF')
             del self.attrib[qn('xsi:foo')]
@@ -135,10 +130,7 @@ def _set_element_text(self, prop_name, value):
         """
         value = str(value)
         if len(value) > 255:
-            tmpl = (
-                "exceeded 255 char limit for property, got:\n\n'%s'"
-            )
-            raise ValueError(tmpl % value)
+            raise ValueError("exceeded 255 char limit for property, got:\n\n'%s'" % value)
         element = self._get_or_add(prop_name)
         element.text = value
 
@@ -153,4 +145,3 @@ def _text_of_element(self, property_name):
         if element.text is None:
             return ''
         return element.text
-

tests/opc/parts/test_customprops.py

@@ -8,23 +8,21 @@
     absolute_import, division, print_function, unicode_literals
 )
 
-from datetime import datetime, timedelta
-
 import pytest
 
 from docx.opc.customprops import CustomProperties
 from docx.opc.parts.customprops import CustomPropertiesPart
 from docx.oxml.customprops import CT_CustomProperties
 
-from ...unitutil.mock import class_mock, instance_mock
+from tests.unitutil.mock import class_mock, instance_mock
 
 
 class DescribeCustomPropertiesPart(object):
 
-    def it_provides_access_to_its_custom_props_object(self, customprops_fixture):
-        custom_properties_part, CustomProperties_ = customprops_fixture
+    def it_provides_access_to_its_custom_props_object(self, element_, mock_custom_properties_):
+        custom_properties_part = CustomPropertiesPart(None, None, element_, None)
         custom_properties = custom_properties_part.custom_properties
-        CustomProperties_.assert_called_once_with(custom_properties_part.element)
+        mock_custom_properties_.assert_called_once_with(custom_properties_part.element)
         assert isinstance(custom_properties, CustomProperties)
 
     def it_can_create_a_default_custom_properties_part(self):
@@ -36,14 +34,7 @@ def it_can_create_a_default_custom_properties_part(self):
     # fixtures ---------------------------------------------
 
     @pytest.fixture
-    def customprops_fixture(self, element_, CustomProperties_):
-        custom_properties_part = CustomPropertiesPart(None, None, element_, None)
-        return custom_properties_part, CustomProperties_
-
-    # fixture components -----------------------------------
-
-    @pytest.fixture
-    def CustomProperties_(self, request):
+    def mock_custom_properties_(self, request):
         return class_mock(request, 'docx.opc.parts.customprops.CustomProperties')
 
     @pytest.fixture