Repository: peopledoc/workalendar
Pull request #428 (2019-11-22 14:58:04)

Changes in the registry.items API
refs #403, #375

Here are the changes in the ``registry.items()`` method API.

- This method is aliased to ``get_calendars()``. In a near release, the ``items()`` method will change its purpose.
- The ``get_calendars()`` method accepts an empty/missing ``region_codes`` argument to retrieve the full registry. Please see the [ISO Registry documentation](https://peopledoc.github.io/workalendar/iso-registry.html) for extensive usage docs.

----

- [x] Tests with a significant number of years to be tested for your calendar.
- [x] Changelog amended with a mention describing your changes.
- [x] Changes in documentation.


Patch:
diff --git a/Changelog.md b/Changelog.md
index 4512be6..7d127e6 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -2,7 +2,11 @@
 
 ## master (unreleased)
 
-Nothing here yet.
+### ISO Registry API Change
+
+- Changes in the ``registry.items()`` method API.
+  - This method is aliased to ``get_calendars()``. In a near release, the ``items()`` method will change its purpose.
+  - The ``get_calendars()`` method accepts an empty/missing ``region_codes`` argument to retrieve the full registry. Please see the [ISO Registry documentation](https://peopledoc.github.io/workalendar/iso-registry.html) for extensive usage docs (#403, #375).
 
 ## v7.2.0 (2019-12-06)
 
diff --git a/docs/iso-registry.md b/docs/iso-registry.md
index 64a798e..e1d6a4f 100644
--- a/docs/iso-registry.md
+++ b/docs/iso-registry.md
@@ -8,7 +8,8 @@ As of version 3.0 (August/September 2018), we have introduced a global calendar
 
 ```python
 >>> from workalendar.registry import registry
->>> for code, calendar_class in registry.region_registry.items():
+>>> calendars = registry.get_calendars()
+>>> for code, calendar_class in calendars.items():
 ...     print("`{}` is code for '{}'".format(code, calendar_class.name))
 `AT` is code for 'Austria'
 `BE` is code for 'Belgium'
@@ -23,14 +24,28 @@ As of version 3.0 (August/September 2018), we have introduced a global calendar
 ... continued
 ```
 
-The `registry.region_registry` is a `dict` object, with the ISO code as a key, and the calendar class as the value. As a "workalendar standard", **every** calendar in the registry has a `name` property (derived from the docstring), so you'd probably be able to build a user-friendly list of available calendars, for a dropdown list, for example.
+The "private property" `registry.region_registry` is a `dict` object, with the ISO code as a key, and the calendar class as the value. As a "workalendar standard", **every** calendar in the registry has a `name` property (derived from the docstring), so you'd probably be able to build a user-friendly list of available calendars, for a dropdown list, for example.
+
+**DEPRECATION WARNING**: the ``get_calendars`` method used to be named ``items()``. In a future release, it'll be deprecated and re-purposed. Please switch to using ``get_calendars()`` for all your queries in the registry.
 
 ## Retrieve a collection of regions
 
-Let's say you'd need only a subset of the ISO registry. For example, France, Switzerland and Canada calendars. You can use the method `items()` to filter only the calendars you want.
+If you want the full dictionary of **countries**, you can use the ``get_calendars()`` method.
+
+```python
+>>> registry.get_calendars()
+{'AT': <class 'workalendar.europe.austria.Austria'>,
+ 'BE': <class 'workalendar.europe.belgium.Belgium'>,
+ 'BG': <class 'workalendar.europe.bulgaria.Bulgaria'>,
+ 'KY': <class 'workalendar.europe.cayman_islands.CaymanIslands'>,
+  # ... continued
+}
+```
+
+Let's say you'd need only a subset of the ISO registry. For example, France, Switzerland and Canada calendars. You can use the method `get_calendars()` to filter only the calendars you want.
 
 ```python
->>> registry.items(['FR', 'CH', 'CA'])
+>>> registry.get_calendars(['FR', 'CH', 'CA'])
 {'FR': <class 'workalendar.europe.france.France'>,
  'CH': <class 'workalendar.europe.switzerland.Switzerland'>,
  'CA': <class 'workalendar.america.canada.Canada'>}
@@ -39,7 +54,7 @@ Let's say you'd need only a subset of the ISO registry. For example, France, Swi
 Also, if you want those regions **and** their subregions, you can use the `include_subregions` flag:
 
 ```python
->>> registry.items(['FR', 'CH', 'CA'], include_subregions=True)
+>>> registry.get_calendars(['FR', 'CH', 'CA'], include_subregions=True)
 {'CA': <class 'workalendar.america.canada.Canada'>,
  'CA-AB': <class 'workalendar.america.canada.Alberta'>,
  'CA-BC': <class 'workalendar.america.canada.BritishColumbia'>,
@@ -62,6 +77,12 @@ Also, if you want those regions **and** their subregions, you can use the `inclu
 
 *Note*: if any of the codes is unknown, this function won't raise an error.
 
+You can also get the full dict of all calendars registered in the ISO Registry with all the subregions using the following function call:
+
+```python
+>>> registry.get_calendars(include_subregions=True)
+```
+
 ## Select only one calendar
 
 Let's say that we only know the ISO code for Switzerland (`CH`). If we want to compute holidays for Switzerland in 2018, we can do as follows:
diff --git a/workalendar/registry.py b/workalendar/registry.py
index c152f6f..f27ef52 100644
--- a/workalendar/registry.py
+++ b/workalendar/registry.py
@@ -2,6 +2,7 @@
 from __future__ import absolute_import, unicode_literals
 
 from importlib import import_module
+import warnings
 
 from .core import Calendar
 from .exceptions import ISORegistryError
@@ -61,13 +62,6 @@ class IsoRegistry(object):
                 if iso_code and cls.__name__ == class_name:
                     self.register(iso_code, cls)
 
-    def _code_elements(self, iso_code):
-        code_elements = iso_code.split('-')
-        is_subregion = False
-        if len(code_elements) > 1:
-            is_subregion = True
-        return code_elements, is_subregion
-
     def get_calendar_class(self, iso_code):
         """
         Retrieve calendar class associated with given ``iso_code``.
@@ -79,14 +73,7 @@ class IsoRegistry(object):
 
         :rtype: Calendar
         """
-        code_elements, is_subregion = self._code_elements(iso_code)
-        if is_subregion and iso_code not in self.region_registry:
-            # subregion code not in region_registry
-            code = code_elements[0]
-        else:
-            # subregion code in region_registry or is not a subregion
-            code = iso_code
-        return self.region_registry.get(code)
+        return self.region_registry.get(iso_code)
 
     def get_subregions(self, iso_code):
         """
@@ -103,22 +90,49 @@ class IsoRegistry(object):
         """
         items = dict()
         for key, value in self.region_registry.items():
-            code_elements, is_subregion = self._code_elements(key)
-            if is_subregion and code_elements[0] == iso_code:
+            if key.startswith("{}-".format(iso_code)):
                 items[key] = value
         return items
 
-    def items(self, region_codes, include_subregions=False):
+    def items(self, region_codes=None, include_subregions=False):
         """
         Returns calendar classes for regions
 
-        :param region_codes list of ISO codes for selected regions
+        :param region_codes list of ISO codes for selected regions. If empty,
+                            the function will return all items from the
+                            registry.
         :param include_subregions boolean if subregions
         of selected regions should be included in result
         :rtype dict
         :return dict where keys are ISO codes strings
         and values are calendar classes
         """
+        warnings.warn("The ``items()`` method will soon be deprecated."
+                      " Please use ``get_calendars()`` instead.",
+                      DeprecationWarning)
+        return self.get_calendars(region_codes, include_subregions)
+
+    def get_calendars(self, region_codes=None, include_subregions=False):
+        """
+        Returns calendar classes for regions
+
+        :param region_codes list of ISO codes for selected regions. If empty,
+                            the function will return all items from the
+                            registry.
+        :param include_subregions boolean if subregions
+        of selected regions should be included in result
+        :rtype dict
+        :return dict where keys are ISO codes strings
+        and values are calendar classes
+        """
+        if not region_codes:
+            # Here it contains all subregions
+            if include_subregions:
+                return self.region_registry.copy()
+            items = {k: v for k, v in self.region_registry.items()
+                     if '-' not in k}
+            return items
+
         items = dict()
         for code in region_codes:
             try:


Test patch:
diff --git a/workalendar/tests/test_registry.py b/workalendar/tests/test_registry.py
index 61f3f60..b716a96 100644
--- a/workalendar/tests/test_registry.py
+++ b/workalendar/tests/test_registry.py
@@ -1,4 +1,5 @@
 from unittest import TestCase
+import warnings
 
 from workalendar.registry import IsoRegistry
 from workalendar.core import Calendar
@@ -17,24 +18,48 @@ class NotACalendarClass(object):
     "Not a Calendar"
 
 
-class MockCalendarTest(TestCase):
+class NonStandardRegistryTest(TestCase):
 
     def setUp(self):
         self.region = RegionCalendar
         self.subregion = SubRegionCalendar
 
-    def test_register(self):
+    def test_region_registry(self):
         registry = IsoRegistry(load_standard_modules=False)
         self.assertEqual(0, len(registry.region_registry.items()))
         registry.register('RE', self.region)
         self.assertEqual(1, len(registry.region_registry.items()))
         self.assertEqual(RegionCalendar, registry.region_registry['RE'])
 
+    def test_register_non_calendar(self):
+        registry = IsoRegistry(load_standard_modules=False)
+        with self.assertRaises(ISORegistryError):
+            registry.register("NAC", NotACalendarClass)
+
     def test_get_calendar_class(self):
         registry = IsoRegistry(load_standard_modules=False)
         registry.register('RE', self.region)
+        registry.register('RE-SR', self.subregion)
         calendar_class = registry.get_calendar_class('RE')
         self.assertEqual(calendar_class, RegionCalendar)
+        # Subregion
+        calendar_class = registry.get_calendar_class('RE-SR')
+        self.assertEqual(calendar_class, SubRegionCalendar)
+        # Unknown code/region
+        self.assertIsNone(registry.get_calendar_class('XX'))
+
+    def test_items_deprecation(self):
+        registry = IsoRegistry(load_standard_modules=False)
+        with warnings.catch_warnings(record=True) as w:
+            # Cause all warnings to always be triggered.
+            warnings.simplefilter("always")
+            # Trigger a warning.
+            registry.items()
+            # Verify some things
+            self.assertEqual(len(w), 1)
+            warning = w[0]
+            self.assertTrue(issubclass(warning.category, DeprecationWarning))
+            self.assertIn("deprecated", str(warning.message))
 
     def test_get_subregions(self):
         registry = IsoRegistry(load_standard_modules=False)
@@ -42,23 +67,85 @@ class MockCalendarTest(TestCase):
         registry.register('RE-SR', self.subregion)
         registry.register('OR-SR', self.subregion)
         subregions = registry.get_subregions('RE')
-        self.assertIn('RE-SR', subregions)
+        # Only one sub-region here
         self.assertEqual(1, len(subregions))
+        self.assertIn('RE-SR', subregions)
 
-    def test_get_items(self):
+    def test_get_calendars(self):
         registry = IsoRegistry(load_standard_modules=False)
         registry.register('RE', self.region)
         registry.register('RE-SR', self.subregion)
         registry.register('OR-SR', self.subregion)
-        items = registry.items(['RE'], include_subregions=True)
-        self.assertEqual(2, len(items))
-        self.assertIn('RE', items)
-        self.assertIn('RE-SR', items)
-        items = registry.items(['RE'], include_subregions=False)
-        self.assertEqual(1, len(items))
-        self.assertIn('RE', items)
+        calendars = registry.get_calendars(['RE'], include_subregions=True)
+        self.assertEqual(2, len(calendars))
+        self.assertIn('RE', calendars)
+        self.assertIn('RE-SR', calendars)
+        calendars = registry.get_calendars(['RE'], include_subregions=False)
+        self.assertEqual(1, len(calendars))
+        self.assertIn('RE', calendars)
 
-    def test_register_non_calendar(self):
+    def test_get_calendars_unknown(self):
         registry = IsoRegistry(load_standard_modules=False)
-        with self.assertRaises(ISORegistryError):
-            registry.register("NAC", NotACalendarClass)
+        registry.register('RE', self.region)
+        calendars = registry.get_calendars(['XX'])
+        self.assertEqual(calendars, {})
+
+    def test_get_calendars_with_subregions(self):
+        registry = IsoRegistry(load_standard_modules=False)
+        registry.register('RE', self.region)
+        registry.register('RE2', self.region)
+        registry.register('RE-SR', self.subregion)
+        calendars = registry.get_calendars(
+            ['RE2', "RE-SR"], include_subregions=True)
+        self.assertEqual(2, len(calendars))
+        self.assertIn('RE2', calendars)
+        self.assertIn('RE-SR', calendars)
+        calendars = registry.get_calendars(
+            ['RE2', "RE-SR"], include_subregions=False)
+        self.assertEqual(2, len(calendars))
+        self.assertIn('RE2', calendars)
+        self.assertIn('RE-SR', calendars)
+
+        # Only a subregion
+        calendars = registry.get_calendars(["RE-SR"], include_subregions=True)
+        self.assertEqual(1, len(calendars))
+        self.assertIn('RE-SR', calendars)
+
+    def test_get_calendars_empty_arg(self):
+        registry = IsoRegistry(load_standard_modules=False)
+        # 3 regions, one sub-region
+        registry.register('RE', self.region)
+        registry.register('RE2', self.region)
+        registry.register('RE3', self.region)
+        registry.register('RE-SR', self.subregion)
+        # Empty arg, no subregions
+        calendars = registry.get_calendars([], include_subregions=False)
+        self.assertEqual(len(calendars), 3)
+        self.assertEqual(set({"RE", "RE2", "RE3"}), set(calendars.keys()))
+        # Empty arg, with subregions
+        calendars = registry.get_calendars([], include_subregions=True)
+        self.assertEqual(len(calendars), 4)
+        self.assertEqual(
+            set({"RE", "RE2", "RE3", "RE-SR"}),
+            set(calendars.keys())
+        )
+
+    def test_get_calendars_no_arg(self):
+        registry = IsoRegistry(load_standard_modules=False)
+        # 3 regions, one sub-region
+        registry.register('RE', self.region)
+        registry.register('RE2', self.region)
+        registry.register('RE3', self.region)
+        registry.register('RE-SR', self.subregion)
+
+        # Should be equivalent to [] + no subregions
+        calendars = registry.get_calendars()
+        self.assertEqual(len(calendars), 3)
+        self.assertEqual(set({"RE", "RE2", "RE3"}), set(calendars.keys()))
+
+        # Should be equivalent to [] + include subregions
+        calendars = registry.items(include_subregions=True)
+        self.assertEqual(len(calendars), 4)
+        self.assertEqual(
+            set({"RE", "RE2", "RE3", "RE-SR"}),
+            set(calendars.keys()))