Repository: peopledoc/workalendar (python)

Issue:
Colombia: refactor class
instead of using flags, it extends the variable days using the ChristianMixin functions directly. We could simplify it.

EDIT: Finally those variable days were legit, because most of them were set to "the next Monday after date X".
I've amended the class & tests.

Pull request description:
Colombia refactor
Small refactoring for the Colombia / added docstrings & comments to clarify exceptions

closes #509

- [x] Tests with a significant number of years to be tested for your calendar.
- [x] Changelog amended with a mention describing your changes. **Note** *Please do NOT change the version number here. It's the project maintainers' duty.*


Patch:
diff --git a/Changelog.md b/Changelog.md
index 84207d9..29007a6 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -2,7 +2,7 @@
 
 ## master (unreleased)
 
-Nothing here yet.
+- Small refactoring for the Colombia / added docstrings & comments to explain why we're not using stock options. Added tests for year 2020 and handling shift exceptions. (#509).
 
 ## v10.3.0 (2020-07-10)
 
diff --git a/workalendar/america/colombia.py b/workalendar/america/colombia.py
index bd39265..a0a68fa 100644
--- a/workalendar/america/colombia.py
+++ b/workalendar/america/colombia.py
@@ -20,43 +20,86 @@ class Colombia(WesternCalendar):
     include_immaculate_conception = True
 
     def get_epiphany(self, year):
+        """
+        Epiphany is shifted in Colombia
+        """
         base_day = date(year, 1, 6)
-        return Colombia.get_first_weekday_after(base_day, MON)
+        return self.get_first_weekday_after(base_day, MON)
 
     def get_saint_joseph(self, year):
         base_day = date(year, 3, 19)
-        return Colombia.get_first_weekday_after(base_day, MON)
+        return self.get_first_weekday_after(base_day, MON)
 
     def get_ascension(self, year):
-        return self.get_easter_sunday(year) + timedelta(days=43)
+        # By default, Ascension falls on THU.
+        # But in Colombia, they celebrate it the next MON.
+        base_day = self.get_ascension_thursday(year)
+        return self.get_first_weekday_after(base_day, MON)
 
     def get_corpus_christi(self, year):
-        return self.get_easter_sunday(year) + timedelta(days=64)
+        # By default, Corpus Christi falls 60 days after Easter.
+        # But in Colombia, they celebrate it the next MON.
+        base_day = super().get_corpus_christi(year)
+        return self.get_first_weekday_after(base_day, MON)
 
     def get_sacred_heart(self, year):
-        return self.get_easter_sunday(year) + timedelta(days=71)
+        # By default, Sacred Heart falls 68 days after Easter.
+        # But in Colombia, they celebrate it the next MON.
+        base_day = self.get_easter_sunday(year) + timedelta(days=68)
+        return self.get_first_weekday_after(base_day, MON)
 
     def get_saint_peter_and_saint_paul(self, year):
         base_day = date(year, 6, 29)
-        return Colombia.get_first_weekday_after(base_day, MON)
+        return self.get_first_weekday_after(base_day, MON)
 
     def get_assumption(self, year):
+        # By default, Assumption is a fixed date (August 15th)
+        # But in Colombia, they celebrate it the next MON.
         base_day = date(year, 8, 15)
-        return Colombia.get_first_weekday_after(base_day, MON)
+        return self.get_first_weekday_after(base_day, MON)
 
-    def get_race_day(self, year):
+    def get_day_of_the_races(self, year):
+        """
+        Return Day of the Races and Hispanity
+
+        a.k.a. "Día de la Raza"
+        Fixed to the next MON after October 12th (Columbus Day)
+        """
         base_day = date(year, 10, 12)
-        return Colombia.get_first_weekday_after(base_day, MON)
+        return self.get_first_weekday_after(base_day, MON)
 
     def get_all_saints(self, year):
+        # By default, All Saints is a fixed date (November 1st)
+        # But in Colombia, they celebrate it the next MON.
         base_day = date(year, 11, 1)
-        return Colombia.get_first_weekday_after(base_day, MON)
+        return self.get_first_weekday_after(base_day, MON)
 
     def get_cartagena_independence(self, year):
+        """
+        Cartagena independance day
+
+        Fixed to the next MON after November 11th.
+        """
         base_day = date(year, 11, 11)
-        return Colombia.get_first_weekday_after(base_day, MON)
+        return self.get_first_weekday_after(base_day, MON)
 
     def get_variable_days(self, year):
+        """
+        Return variable holidays for Colombia
+
+        The following days are set to "the next MON after the 'true' date".
+
+        * Epiphany,
+        * Saint Joseph,
+        * Ascension,
+        * Corpus Christi,
+        * Sacred Heart,
+        * Saint Peter & Saint Paul
+        * Assumption
+        * Columbus Day / Race Day
+        * All Saints
+        * Cartagena Independance
+        """
         days = super().get_variable_days(year)
         days.extend([
             (self.get_epiphany(year), "Epiphany"),
@@ -66,7 +109,8 @@ class Colombia(WesternCalendar):
             (self.get_saint_peter_and_saint_paul(year),
                 "Saint Peter and Saint Paul"),
             (self.get_assumption(year), "Assumption of Mary to Heaven"),
-            (self.get_race_day(year), "Race Day"),
+            (self.get_day_of_the_races(year),
+                "Day of the Races and Hispanity"),
             (self.get_all_saints(year), "All Saints"),
             (self.get_cartagena_independence(year),
                 "Cartagena's Independence"),


Test patch:
diff --git a/workalendar/tests/test_america.py b/workalendar/tests/test_america.py
index bbea135..1921b67 100644
--- a/workalendar/tests/test_america.py
+++ b/workalendar/tests/test_america.py
@@ -163,28 +163,100 @@ class ColombiaTest(GenericCalendarTest):
 
     def test_holidays_2015(self):
         holidays = self.cal.holidays_set(2015)
-        self.assertIn(date(2015, 1, 1), holidays)
-        self.assertIn(date(2015, 1, 12), holidays)
-        self.assertIn(date(2015, 3, 23), holidays)
-        self.assertIn(date(2015, 3, 29), holidays)
-        self.assertIn(date(2015, 4, 2), holidays)
-        self.assertIn(date(2015, 4, 3), holidays)
-        self.assertIn(date(2015, 4, 5), holidays)
-        self.assertIn(date(2015, 5, 1), holidays)
-        self.assertIn(date(2015, 5, 18), holidays)
-        self.assertIn(date(2015, 6, 8), holidays)
-        self.assertIn(date(2015, 6, 15), holidays)
-        self.assertIn(date(2015, 6, 29), holidays)
-        self.assertIn(date(2015, 7, 20), holidays)
-        self.assertIn(date(2015, 8, 7), holidays)
-        self.assertIn(date(2015, 8, 17), holidays)
-        self.assertIn(date(2015, 10, 12), holidays)
-        self.assertIn(date(2015, 11, 2), holidays)
-        self.assertIn(date(2015, 11, 16), holidays)
-        self.assertIn(date(2015, 12, 8), holidays)
-        self.assertIn(date(2015, 12, 25), holidays)
+        self.assertIn(date(2015, 1, 1), holidays)    # New year
+        self.assertIn(date(2015, 1, 12), holidays)   # Epiphany (shifted)
+        self.assertIn(date(2015, 3, 23), holidays)   # Saint Joseph
+        self.assertIn(date(2015, 3, 29), holidays)   # Palm Sunday
+        self.assertIn(date(2015, 4, 2), holidays)    # Holy Thursday
+        self.assertIn(date(2015, 4, 3), holidays)    # Good Friday
+        self.assertIn(date(2015, 4, 5), holidays)    # Easter (SUN)
+        self.assertIn(date(2015, 5, 1), holidays)    # Labour Day
+        self.assertIn(date(2015, 5, 18), holidays)   # Ascension (shifted)
+        self.assertIn(date(2015, 6, 8), holidays)    # Corpus Christi
+        self.assertIn(date(2015, 6, 15), holidays)   # Sacred Heart
+        self.assertIn(date(2015, 6, 29), holidays)   # St Peter & St Paul
+        self.assertIn(date(2015, 7, 20), holidays)   # Independance Day
+        self.assertIn(date(2015, 8, 7), holidays)    # Boyacá battle
+        self.assertIn(date(2015, 8, 17), holidays)   # Assumption (shifted)
+        self.assertIn(date(2015, 10, 12), holidays)  # Day of the Races
+        self.assertIn(date(2015, 11, 2), holidays)   # All Saints (shifted)
+        self.assertIn(date(2015, 11, 16), holidays)  # Cartagena independence
+        self.assertIn(date(2015, 12, 8), holidays)   # Immaculate Conception
+        self.assertIn(date(2015, 12, 25), holidays)  # XMas
         self.assertEqual(len(holidays), 20)
 
+    def test_holidays_2020(self):
+        holidays = self.cal.holidays_set(2020)
+        self.assertIn(date(2020, 1, 1), holidays)    # New year
+        self.assertIn(date(2020, 1, 6), holidays)    # Epiphany
+        self.assertIn(date(2020, 3, 23), holidays)   # Saint Joseph
+        self.assertIn(date(2020, 4, 5), holidays)    # Palm Sunday
+        self.assertIn(date(2020, 4, 9), holidays)    # Holy Thursday
+        self.assertIn(date(2020, 4, 10), holidays)   # Good Friday
+        self.assertIn(date(2020, 4, 12), holidays)   # Easter (SUN)
+        self.assertIn(date(2020, 5, 1), holidays)    # Labour Day
+        self.assertIn(date(2020, 5, 25), holidays)   # Ascension (shifted)
+        self.assertIn(date(2020, 6, 15), holidays)   # Corpus Christi
+        self.assertIn(date(2020, 6, 22), holidays)   # Sacred Heart
+        self.assertIn(date(2020, 6, 29), holidays)   # St Peter & St Paul
+        self.assertIn(date(2020, 7, 20), holidays)   # Independance Day
+        self.assertIn(date(2020, 8, 7), holidays)    # Boyacá battle
+        self.assertIn(date(2020, 8, 17), holidays)   # Assumption (shifted)
+        self.assertIn(date(2020, 10, 12), holidays)  # Day of the Races
+        self.assertIn(date(2020, 11, 2), holidays)   # All Saints (shifted)
+        self.assertIn(date(2020, 11, 16), holidays)  # Cartagena independence
+        self.assertIn(date(2020, 12, 8), holidays)   # Immaculate Conception
+        self.assertIn(date(2020, 12, 25), holidays)  # XMas
+        self.assertEqual(len(holidays), 20)
+
+    def test_epiphany_monday(self):
+        # In 2020, Epiphany falls on MON
+        epiphany_2020 = self.cal.get_epiphany(2020)
+        self.assertEqual(epiphany_2020, date(2020, 1, 6))
+        # In 2021, it does not, so it's shifted to the next MON
+        epiphany_2021 = self.cal.get_epiphany(2021)
+        self.assertEqual(epiphany_2021, date(2021, 1, 11))
+
+    def test_saint_peter_and_saint_paul_monday(self):
+        # In 2020, Saint Peter and Saint Paul falls on MON
+        st_peter_paul_2020 = self.cal.get_saint_peter_and_saint_paul(2020)
+        self.assertEqual(st_peter_paul_2020, date(2020, 6, 29))
+        # In 2021, it does not, so it's shifted to the next MON
+        st_peter_paul_2021 = self.cal.get_saint_peter_and_saint_paul(2021)
+        self.assertEqual(st_peter_paul_2021, date(2021, 7, 5))
+
+    def test_assumption_monday(self):
+        # In 2021, Assumption falls on SUN, so it's shifted to MON
+        assumption_2021 = self.cal.get_assumption(2021)
+        self.assertEqual(assumption_2021, date(2021, 8, 16))
+        # In 2022, Assumption falls on MON
+        assumption_2022 = self.cal.get_assumption(2022)
+        self.assertEqual(assumption_2022, date(2022, 8, 15))
+
+    def test_day_of_the_races_monday(self):
+        # In 2020, Day of the races and hispanity falls on MON
+        day_races_2020 = self.cal.get_day_of_the_races(2020)
+        self.assertEqual(day_races_2020, date(2020, 10, 12))
+        # In 2021, It does not, so it's shifted to the next MON
+        day_races_2021 = self.cal.get_day_of_the_races(2021)
+        self.assertEqual(day_races_2021, date(2021, 10, 18))
+
+    def test_all_saints_monday(self):
+        # In 2021, The All Saints falls on MON
+        all_saints_2021 = self.cal.get_all_saints(2021)
+        self.assertEqual(all_saints_2021, date(2021, 11, 1))
+        # In 2022, It does not, so it's shifted to the next MON
+        all_saints_2022 = self.cal.get_all_saints(2022)
+        self.assertEqual(all_saints_2022, date(2022, 11, 7))
+
+    def test_cartagena_independence_monday(self):
+        # In 2019, The Cartagena Independance falls on MON
+        cartagena_2019 = self.cal.get_cartagena_independence(2019)
+        self.assertEqual(cartagena_2019, date(2019, 11, 11))
+        # In 2020, It does not, so it's shifted to the next MON
+        cartagena_2020 = self.cal.get_cartagena_independence(2020)
+        self.assertEqual(cartagena_2020, date(2020, 11, 16))
+
 
 class MexicoTest(GenericCalendarTest):
     cal_class = Mexico