jenkins-bot has submitted this change and it was merged.

Change subject: [FEAT] NamespacesDict to replace Namespace methods ......................................................................

[FEAT] NamespacesDict to replace Namespace methods

Instead of having static/classmethods in Namespace use a dict which stores the Namespace instances and allows general handling of them. This reuses the _NamespacesDict introduced in ae69d8dc. This makes doing lookups easier as it's not necessary to import the Namespace class and having to manually define on which namespaces it works.

This makes also ns_index unnecessary as it's relatively easy possible to actually get the Namespace using the name and a site instance now.

It also adds a new TestCase class which patches assert calls to easily check if the deprecation message was issued after each assertion.

Change-Id: I74507af6f22cf09fa161f5df07e1dd90cb399759 --- M pywikibot/data/api.py M pywikibot/page.py M pywikibot/pagegenerators.py M pywikibot/site.py M scripts/templatecount.py M tests/aspects.py M tests/namespace_tests.py M tests/site_tests.py M tests/wikibase_tests.py 9 files changed, 254 insertions(+), 116 deletions(-)

Approvals: John Vandenberg: Looks good to me, approved jenkins-bot: Verified

diff --git a/pywikibot/data/api.py b/pywikibot/data/api.py index 3e5d960..9549e46 100644 --- a/pywikibot/data/api.py +++ b/pywikibot/data/api.py @@ -2348,8 +2348,7 @@

# Use Namespace id (int) here; Request will cast int to str namespaces = [ns.id for ns in - pywikibot.site.Namespace.resolve(namespaces, - self.site.namespaces)] + self.site.namespaces.resolve(namespaces)]

if 'multi' not in param and len(namespaces) != 1: raise TypeError(u'{0} module does not support multiple namespaces' diff --git a/pywikibot/page.py b/pywikibot/page.py index b47badd..69a1c02 100644 --- a/pywikibot/page.py +++ b/pywikibot/page.py @@ -4678,7 +4678,7 @@ t = t.lstrip(u":").lstrip(u" ") continue prefix = t[:t.index(u":")].lower() # part of text before : - ns = self._source.ns_index(prefix) + ns = self._source.namespaces.lookup_name(prefix) if ns: # The prefix is a namespace in the source wiki return (fam.name, code) @@ -4719,7 +4719,7 @@ continue

prefix = t[:t.index(u":")].lower() - ns = self._site.ns_index(prefix) + ns = self._site.namespaces.lookup_name(prefix) if ns: # Ordinary namespace t = t[t.index(u":"):].lstrip(u":").lstrip(u" ") @@ -4765,7 +4765,7 @@ self._namespace + 1] if '' in other_ns: # other namespace uses empty str as ns next_ns = t[:t.index(':')] - if self._site.ns_index(next_ns): + if self._site.namespaces.lookup_name(next_ns): raise pywikibot.InvalidTitle( u"The (non-)talk page of '{0}' is a valid title " "in another namespace.".format(self._text)) @@ -4890,7 +4890,7 @@ else: # look for corresponding ns in onsite by name comparison for alias in ns: - namespace = Namespace.lookup_name(alias, onsite.namespaces) + namespace = onsite.namespaces.lookup_name(alias) if namespace: namespace = namespace.custom_name break @@ -5018,7 +5018,7 @@

if ':' in title: ns, t = title.split(':', 1) - ns = link._site.ns_index(ns.lower()) + ns = link._site.namespaces.lookup_name(ns) if ns: link._namespace = ns title = t diff --git a/pywikibot/pagegenerators.py b/pywikibot/pagegenerators.py index df3382c..fe51ab1 100644 --- a/pywikibot/pagegenerators.py +++ b/pywikibot/pagegenerators.py @@ -47,7 +47,6 @@ from pywikibot.comms import http from pywikibot.data import wikidataquery as wdquery from pywikibot.exceptions import ArgumentDeprecationWarning -from pywikibot.site import Namespace

if sys.version_info[0] > 2: basestring = (str, ) @@ -355,7 +354,7 @@ """ if isinstance(self._namespaces, list): self._namespaces = frozenset( - Namespace.resolve(self._namespaces, self.site.namespaces)) + self.site.namespaces.resolve(self._namespaces)) return self._namespaces

def getCombinedGenerator(self, gen=None): @@ -1208,10 +1207,10 @@ # As site was only required if the namespaces contain strings, dont # attempt to use the config selected site unless the initial attempt # at resolving the namespaces fails. + if not site: + site = pywikibot.Site() try: - namespaces = Namespace.resolve(namespaces, - site.namespaces if site else - pywikibot.Site().namespaces) + namespaces = site.namespaces.resolve(namespaces) except KeyError as e: pywikibot.log('Failed resolving namespaces:') pywikibot.exception(e) diff --git a/pywikibot/site.py b/pywikibot/site.py index c4d53ce..b361d61 100644 --- a/pywikibot/site.py +++ b/pywikibot/site.py @@ -26,13 +26,13 @@ import copy import mimetypes

-from collections import Iterable, Container, namedtuple +from collections import Iterable, Container, namedtuple, Mapping from warnings import warn

import pywikibot import pywikibot.family from pywikibot.tools import ( - itergroup, UnicodeMixin, ComparableMixin, SelfCallDict, SelfCallString, + itergroup, UnicodeMixin, ComparableMixin, SelfCallMixin, SelfCallString, deprecated, deprecate_arg, deprecated_args, remove_last_args, redirect_func, issue_deprecation_warning, manage_wrapping, MediaWikiVersion, first_upper, normalize_username, @@ -414,6 +414,7 @@ return False

@classmethod + @deprecated('NamespacesDict.lookup_name') def lookup_name(cls, name, namespaces=None): """Find the Namespace for a name.

@@ -427,18 +428,10 @@ if not namespaces: namespaces = cls.builtin_namespaces()

- name = cls.normalize_name(name) - if name is False: - return None - name = name.lower() - - for namespace in namespaces.values(): - if namespace._contains_lowercase_name(name): - return namespace - - return None + return NamespacesDict._lookup_name(name, namespaces)

@staticmethod + @deprecated('NamespacesDict.resolve') def resolve(identifiers, namespaces=None): """ Resolve namespace identifiers to obtain Namespace objects. @@ -461,6 +454,83 @@ if not namespaces: namespaces = Namespace.builtin_namespaces()

+ return NamespacesDict._resolve(identifiers, namespaces) + + +class NamespacesDict(Mapping, SelfCallMixin): + + """ + An immutable dictionary containing the Namespace instances. + + It adds a deprecation message when called as the 'namespaces' property of + APISite was callable. + """ + + _own_desc = 'the namespaces property' + + def __init__(self, namespaces): + """Create new dict using the given namespaces.""" + super(NamespacesDict, self).__init__() + self._namespaces = namespaces + + def __iter__(self): + """Iterate over all namespaces.""" + return iter(self._namespaces) + + def __getitem__(self, number): + """Get the namespace with the given number.""" + return self._namespaces[number] + + def __len__(self): + """Get the number of namespaces.""" + return len(self._namespaces) + + def lookup_name(self, name): + """ + Find the Namespace for a name also checking aliases. + + @param name: Name of the namespace. + @type name: basestring + @return: Namespace or None + """ + return self._lookup_name(name, self._namespaces) + + # Temporary until Namespace.lookup_name can be removed + @staticmethod + def _lookup_name(name, namespaces): + name = Namespace.normalize_name(name) + if name is False: + return None + name = name.lower() + + for namespace in namespaces.values(): + if namespace._contains_lowercase_name(name): + return namespace + + return None + + def resolve(self, identifiers): + """ + Resolve namespace identifiers to obtain Namespace objects. + + Identifiers may be any value for which int() produces a valid + namespace id, except bool, or any string which Namespace.lookup_name + successfully finds. A numerical string is resolved as an integer. + + @param identifiers: namespace identifiers + @type identifiers: iterable of basestring or Namespace key, + or a single instance of those types + @return: list of Namespace objects in the same order as the + identifiers + @raises KeyError: a namespace identifier was not resolved + @raises TypeError: a namespace identifier has an inappropriate + type such as NoneType or bool + """ + return self._resolve(identifiers, self._namespaces) + + # Temporary until Namespace.resolve can be removed + @staticmethod + def _resolve(identifiers, namespaces): if isinstance(identifiers, (basestring, Namespace)): identifiers = [identifiers] else: @@ -473,7 +543,7 @@ # lookup namespace names, and assume anything else is a key. # int(None) raises TypeError; however, bool needs special handling. result = [NotImplemented if isinstance(ns, bool) else - Namespace.lookup_name(ns, namespaces) + NamespacesDict._lookup_name(ns, namespaces) if isinstance(ns, basestring) and not ns.lstrip('-').isdigit() else namespaces[int(ns)] if int(ns) in namespaces @@ -492,14 +562,6 @@ if ns is None]))

return result - - -# This class can be removed after self-callability has been removed -class _NamespacesDict(SelfCallDict): - - """A wrapper to add a deprecation message when called.""" - - _own_desc = 'the namespaces property'

class BaseSite(ComparableMixin): @@ -787,6 +849,7 @@ self.interwiki(prefix) return self._iw_sites[prefix][1]

+ @deprecated('APISite.namespaces.lookup_name') def ns_index(self, namespace): """ Return the Namespace for a given namespace name. @@ -796,11 +859,12 @@ @return: The matching Namespace object on this Site @rtype: Namespace, or None if invalid """ - return Namespace.lookup_name(namespace, self.namespaces) + return self.namespaces.lookup_name(namespace)

- # for backwards-compatibility - getNamespaceIndex = redirect_func(ns_index, old_name='getNamespaceIndex', - class_name='BaseSite') + @deprecated('APISite.namespaces.lookup_name') + def getNamespaceIndex(self, namespace): + """DEPRECATED: Return the Namespace for a given namespace name.""" + return self.namespaces.lookup_name(namespace)

def _build_namespaces(self): """Create default namespaces.""" @@ -811,7 +875,7 @@ def namespaces(self): """Return dict of valid namespaces on this wiki.""" if not hasattr(self, '_namespaces'): - self._namespaces = _NamespacesDict(self._build_namespaces()) + self._namespaces = NamespacesDict(self._build_namespaces()) return self._namespaces

def ns_normalize(self, value): @@ -821,7 +885,7 @@ @type value: unicode

""" - index = self.ns_index(value) + index = self.namespaces.lookup_name(value) return self.namespace(index)

# for backwards-compatibility @@ -938,7 +1002,7 @@ """Separate the namespace from the name.""" ns, delim, name = title.partition(':') if delim: - ns = Namespace.lookup_name(ns, self.namespaces) + ns = self.namespaces.lookup_name(ns) if not delim or not ns: return default_ns, title else: diff --git a/scripts/templatecount.py b/scripts/templatecount.py index 5751641..f875627 100755 --- a/scripts/templatecount.py +++ b/scripts/templatecount.py @@ -98,7 +98,7 @@ mysite = pywikibot.Site() # The names of the templates are the keys, and lists of pages # transcluding templates are the values. - mytpl = mysite.ns_index(mysite.template_namespace()) + mytpl = mysite.namespaces.lookup_name(mysite.template_namespace()) for template in templates: transcludingArray = [] gen = pywikibot.Page(mysite, template, ns=mytpl).getReferences( diff --git a/tests/aspects.py b/tests/aspects.py index 3e6f6c2..8bd4553 100644 --- a/tests/aspects.py +++ b/tests/aspects.py @@ -37,6 +37,8 @@ import time import warnings

+from contextlib import contextmanager + import pywikibot

from pywikibot import config, log, ServerError, Site @@ -937,6 +939,58 @@ return page

+class CapturingTestCase(TestCase): + + """ + Capture assertion calls to do additional calls around them. + + All assertions done which start with "assert" are patched in such a way that + after the assertion it calls C{process_assertion} with the assertion and the + arguments. + + To avoid that it patches the assertion it's possible to put the call in an + C{disable_assert_capture} with-statement. + + """ + + # Is True while an assertion is running, so that assertions won't be patched + # when they are executed while an assertion is running and only the outer + # most assertion gets actually patched. + _patched = False + + @contextmanager + def disable_assert_capture(self): + """A context manager which preventing that asssertions are patched.""" + nested = self._patched # Don't reset if it was set before + self._patched = True + yield + if not nested: + self._patched = False + + def process_assert(self, assertion, *args, **kwargs): + """Handle the assertion.""" + assertion(*args, **kwargs) + + def patch_assert(self, assertion): + """Execute process_assert when the assertion is called.""" + def inner_assert(*args, **kwargs): + assert self._patched is False + self._patched = True + try: + self.process_assert(assertion, *args, **kwargs) + finally: + self._patched = False + return inner_assert + + def __getattribute__(self, attr): + """Patch assertions if enabled.""" + result = super(CapturingTestCase, self).__getattribute__(attr) + if attr.startswith('assert') and not self._patched: + return self.patch_assert(result) + else: + return result + + class SiteAttributeTestCase(TestCase):

"""Add the sites as attributes to the instances.""" @@ -1273,6 +1327,13 @@ if self._do_test_warning_filename: self.assertDeprecationFile(self.expect_warning_filename)

+ def assertOneDeprecation(self, msg=None, reset=True): + """Assert that exactly one deprecation happened and reset if wished.""" + self.assertEqual(len(self.deprecation_messages), 1) + self.assertDeprecation(msg) + if reset: + self._reset_messages() + def assertNoDeprecation(self, msg=None): if msg: self.assertNotIn(msg, self.deprecation_messages) @@ -1299,3 +1360,29 @@ self.context_manager.__exit__()

super(DeprecationTestCase, self).tearDown() + + +class AutoDeprecationTestCase(CapturingTestCase, DeprecationTestCase): + + """ + A test case capturing asserts and asserting a deprecation afterwards. + + For example C{assertEqual} will do first C{assertEqual} and then + C{assertOneDeprecation}. + + With C{check_file} it's possible to enable or disable the check whether the + filename matches the caller's filename. By default it does not match the + filename if the name starts with 'assertRaises' (as it'll call it inside the + assert in that case). + """ + + def process_assert(self, assertion, *args, **kwargs): + """Handle assertion and call C{assertOneDeprecation} after it.""" + super(AutoDeprecationTestCase, self).process_assert( + assertion, *args, **kwargs) + self._do_test_warning_filename = self.check_file(assertion.__name__) + self.assertOneDeprecation() + + def check_file(self, name): + """Disable filename check on asserted exceptions.""" + return not name.startswith('assertRaises') diff --git a/tests/namespace_tests.py b/tests/namespace_tests.py index 6f18a73..1638114 100644 --- a/tests/namespace_tests.py +++ b/tests/namespace_tests.py @@ -11,12 +11,39 @@

from collections import Iterable from pywikibot.site import Namespace -from tests.aspects import unittest, TestCase +from tests.aspects import unittest, TestCase, AutoDeprecationTestCase

import sys if sys.version_info[0] > 2: basestring = (str, ) unicode = str + +# Default namespaces which should work in any MW wiki +_base_builtin_ns = { + 'Media': -2, + 'Special': -1, + '': 0, + 'Talk': 1, + 'User': 2, + 'User talk': 3, + 'Project': 4, + 'Project talk': 5, + 'MediaWiki': 8, + 'MediaWiki talk': 9, + 'Template': 10, + 'Template talk': 11, + 'Help': 12, + 'Help talk': 13, + 'Category': 14, + 'Category talk': 15, +} +image_builtin_ns = dict(_base_builtin_ns) +image_builtin_ns['Image'] = 6 +image_builtin_ns['Image talk'] = 7 +file_builtin_ns = dict(_base_builtin_ns) +file_builtin_ns['File'] = 6 +file_builtin_ns['File talk'] = 7 +builtin_ns = dict(list(image_builtin_ns.items()) + list(file_builtin_ns.items()))

class TestNamespaceObject(TestCase): @@ -24,35 +51,6 @@ """Test cases for Namespace class."""

net = False - - # These should work in any MW wiki - builtin_ids = { - 'Media': -2, - 'Special': -1, - '': 0, - 'Talk': 1, - 'User': 2, - 'User talk': 3, - 'Project': 4, - 'Project talk': 5, - 'File': 6, - 'File talk': 7, - 'MediaWiki': 8, - 'MediaWiki talk': 9, - 'Template': 10, - 'Template talk': 11, - 'Help': 12, - 'Help talk': 13, - 'Category': 14, - 'Category talk': 15, - } - - old_builtin_ids = { - 'Image': 6, - 'Image talk': 7, - } - - all_builtin_ids = dict(list(builtin_ids.items()) + list(old_builtin_ids.items()))

def testNamespaceTypes(self): """Test cases for methods manipulating namespace names.""" @@ -68,20 +66,6 @@ self.assertTrue(all(isinstance(name, basestring) for val in ns.values() for name in val)) - - self.assertTrue(all(isinstance(Namespace.lookup_name(b, ns), Namespace) - for b in self.builtin_ids)) - - self.assertTrue(all(Namespace.lookup_name(b, ns).id == self.all_builtin_ids[b] - for b in self.all_builtin_ids)) - - ns = Namespace.builtin_namespaces(use_image_name=True) - - self.assertTrue(all(isinstance(Namespace.lookup_name(b, ns), Namespace) - for b in self.builtin_ids)) - - self.assertTrue(all(Namespace.lookup_name(b, ns).id == self.all_builtin_ids[b] - for b in self.all_builtin_ids))

# Use a namespace object as a dict key self.assertEqual(ns[ns[6]], ns[6]) @@ -218,6 +202,13 @@ b = eval(repr(a)) self.assertEqual(a, b)

+ +class TestNamespaceDictDeprecated(AutoDeprecationTestCase): + + """Test static/classmethods in Namespace replaced by NamespacesDict.""" + + net = False + def test_resolve(self): """Test Namespace.resolve.""" namespaces = Namespace.builtin_namespaces(use_image_name=False) @@ -276,6 +267,20 @@ r'Namespace identifier(s) not recognised: -10,-11', Namespace.resolve, [-10, 0, -11])

+ def test_lookup_name(self): + """Test Namespace.lookup_name.""" + file_nses = Namespace.builtin_namespaces(use_image_name=False) + image_nses = Namespace.builtin_namespaces(use_image_name=True) + + for name, ns_id in builtin_ns.items(): + file_ns = Namespace.lookup_name(name, file_nses) + self.assertIsInstance(file_ns, Namespace) + image_ns = Namespace.lookup_name(name, image_nses) + self.assertIsInstance(image_ns, Namespace) + with self.disable_assert_capture(): + self.assertEqual(file_ns.id, ns_id) + self.assertEqual(image_ns.id, ns_id) +

class TestNamespaceCollections(TestCase):

diff --git a/tests/site_tests.py b/tests/site_tests.py index 7b25577..e5497d8 100644 --- a/tests/site_tests.py +++ b/tests/site_tests.py @@ -12,7 +12,7 @@

import sys import os -from collections import Iterable +from collections import Iterable, Mapping from datetime import datetime import re

@@ -120,6 +120,11 @@ for page in self.site.allpages(filterredir='', total=1): self.assertFalse(page.isRedirectPage()) self.assertDeprecation() + + def test_ns_index(self): + """Test ns_index.""" + self.assertEqual(self.site.ns_index('MediaWiki'), 8) + self.assertOneDeprecation()

class TestSiteDryDeprecatedFunctions(DefaultDrySiteTestCase, DeprecationTestCase): @@ -257,28 +262,8 @@ def testNamespaceMethods(self): """Test cases for methods manipulating namespace names.""" mysite = self.get_site() - builtins = { - '': 0, # these should work in any MW wiki - 'Talk': 1, - 'User': 2, - 'User talk': 3, - 'Project': 4, - 'Project talk': 5, - 'Image': 6, - 'Image talk': 7, - 'MediaWiki': 8, - 'MediaWiki talk': 9, - 'Template': 10, - 'Template talk': 11, - 'Help': 12, - 'Help talk': 13, - 'Category': 14, - 'Category talk': 15, - } - self.assertTrue(all(mysite.ns_index(b) == builtins[b] - for b in builtins)) ns = mysite.namespaces - self.assertIsInstance(ns, dict) + self.assertIsInstance(ns, Mapping) self.assertTrue(all(x in ns for x in range(0, 16))) # built-in namespaces always present self.assertIsInstance(mysite.ns_normalize("project"), basestring) @@ -1006,7 +991,7 @@ title = change['title'] self.assertIn(":", title) prefix = title[:title.index(":")] - self.assertIn(mysite.ns_index(prefix), [6, 7]) + self.assertIn(self.site.namespaces.lookup_name(prefix).id, [6, 7]) self.assertIn(change["ns"], [6, 7]) if MediaWikiVersion(mysite.version()) <= MediaWikiVersion("1.14"): pagelist = [mainpage] @@ -1248,7 +1233,7 @@ title = rev['title'] self.assertIn(":", title) prefix = title[:title.index(":")] - self.assertIn(mysite.ns_index(prefix), [6, 7]) + self.assertIn(self.site.namespaces.lookup_name(prefix).id, [6, 7]) self.assertIn(rev["ns"], [6, 7]) for rev in mysite.watchlist_revs(showMinor=True, total=5): self.assertIsInstance(rev, dict) @@ -2525,7 +2510,7 @@ site = self.get_site('wikia') self.assertEqual(site.hostname(), 'www.wikia.com') self.assertEqual(site.code, 'wikia') - self.assertIsInstance(site.namespaces, dict) + self.assertIsInstance(site.namespaces, Mapping) self.assertFalse(site.obsolete) self.assertEqual(site.family.hostname('en'), 'www.wikia.com') self.assertEqual(site.family.hostname('wikia'), 'www.wikia.com') @@ -2558,7 +2543,7 @@ site = self.get_site('lyricwiki') self.assertEqual(site.hostname(), 'lyrics.wikia.com') self.assertEqual(site.code, 'en') - self.assertIsInstance(site.namespaces, dict) + self.assertIsInstance(site.namespaces, Mapping) self.assertFalse(site.obsolete) self.assertEqual(site.family.hostname('en'), 'lyrics.wikia.com')

@@ -2576,7 +2561,7 @@ site = self.get_site('commons') self.assertEqual(site.hostname(), 'commons.wikimedia.org') self.assertEqual(site.code, 'commons') - self.assertIsInstance(site.namespaces, dict) + self.assertIsInstance(site.namespaces, Mapping) self.assertFalse(site.obsolete)

self.assertRaises(KeyError, site.family.hostname, 'en') @@ -2608,7 +2593,7 @@ site = self.get_site('wikidata') self.assertEqual(site.hostname(), 'www.wikidata.org') self.assertEqual(site.code, 'wikidata') - self.assertIsInstance(site.namespaces, dict) + self.assertIsInstance(site.namespaces, Mapping) self.assertFalse(site.obsolete)

self.assertRaises(KeyError, site.family.hostname, 'en') diff --git a/tests/wikibase_tests.py b/tests/wikibase_tests.py index bbac998..0c9bb28 100644 --- a/tests/wikibase_tests.py +++ b/tests/wikibase_tests.py @@ -17,9 +17,8 @@ import pywikibot

from pywikibot import pagegenerators -from pywikibot.tools import SelfCallDict from pywikibot.page import WikibasePage, ItemPage -from pywikibot.site import Namespace +from pywikibot.site import Namespace, NamespacesDict

from tests.aspects import ( unittest, TestCase, @@ -842,7 +841,7 @@ def setUpClass(cls): super(TestAlternateNamespaces, cls).setUpClass()

- cls.get_repo()._namespaces = SelfCallDict({ + cls.get_repo()._namespaces = NamespacesDict({ 90: Namespace(id=90, case='first-letter', canonical_name='Item',