Repository: crsmithdev/arrow
Pull request #768 (2020-04-04 15:06:50)

Consolidate German locales and add missing 'week'/'weeks' localization
German locales are missing translations for the week/weeks granularities.

While working on that, I noticed that `SwissLocale` has them and that it declared itself responsible for plain `de` (as well as `GermanLocale`), so it's a question of chance which one is used to localize for `de` (as opposed to `de_de` or `de_ch`). As I couldn't spot an effective difference between the two, this PR merges both into one class (for `de`, `de_ch`, `de_de`). Additionally, I renamed `DeutschBaseLocale` to `GermanBaseLocale` for consistency and, because its purpose wasn't clear to me right away, straightened out the custom `describe()` method.

/cc @href You might want to take a look at this, please check if I missed anything regarding the equivalence of `GermanLocale` and `SwissLocale`.

## Pull Request Checklist

Thank you for taking the time to improve Arrow! Before submitting your pull request, please check all *appropriate* boxes:

<!-- Check boxes by placing an x in the brackets: [x] -->
- [x] 🧪 Added **tests** for changed code.
- [x] 🛠️ All tests **pass** when run locally (run `tox` or `make test` to find out!).
- [x] 📚 Updated **documentation** for changed code.
- [x] ⏩ Code is **up-to-date** with the `master` branch.

If you have *any* questions about your code changes or any of the points above, please submit your questions along with the pull request and we will try our best to help!

## Description of Changes

* Add week/weeks granularity to German locales.
* Rename `DeutschBaseLocale` to `GermanBaseLocale` for consistency.
* Consolidate `GermanLocale` and `SwissLocale` under the former as they seem to be equivalent.
* Clarify purpose `GermanBaseLocale.describe()` and optimize.

codecov-io: # [Codecov](https://codecov.io/gh/crsmithdev/arrow/pull/768?src=pr&el=h1) Report
> Merging [#768](https://codecov.io/gh/crsmithdev/arrow/pull/768?src=pr&el=desc) into [master](https://codecov.io/gh/crsmithdev/arrow/commit/34213790ebbafb743ddcc8fb15d051bc9bd44249&el=desc) will **not change** coverage by `%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/crsmithdev/arrow/pull/768/graphs/tree.svg?width=650&height=150&src=pr&token=4EkGnFsIGn)](https://codecov.io/gh/crsmithdev/arrow/pull/768?src=pr&el=tree)

```diff
@@            Coverage Diff            @@
##            master      #768   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           10        10           
  Lines         1696      1672   -24     
  Branches       288       288           
=========================================
- Hits          1696      1672   -24     
```


| [Impacted Files](https://codecov.io/gh/crsmithdev/arrow/pull/768?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [arrow/locales.py](https://codecov.io/gh/crsmithdev/arrow/pull/768/diff?src=pr&el=tree#diff-YXJyb3cvbG9jYWxlcy5weQ==) | `100.00% <100.00%> (ø)` | |
| [arrow/arrow.py](https://codecov.io/gh/crsmithdev/arrow/pull/768/diff?src=pr&el=tree#diff-YXJyb3cvYXJyb3cucHk=) | `100.00% <0.00%> (ø)` | |
| [arrow/parser.py](https://codecov.io/gh/crsmithdev/arrow/pull/768/diff?src=pr&el=tree#diff-YXJyb3cvcGFyc2VyLnB5) | `100.00% <0.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/crsmithdev/arrow/pull/768?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/crsmithdev/arrow/pull/768?src=pr&el=footer). Last update [3421379...b0cff82](https://codecov.io/gh/crsmithdev/arrow/pull/768?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

href: Thanks for pinging me. Unfortunately, I switched jobs and am not involved with arrow in any capacity anymore.

However, I'll ping @DadaDaMotha, whose tests are almost certainly going to fail should this pull request be merged and released, as the `SwissLocale` class is used explicitly.

He should be able to confirm if this looks good from his end and I'm sure he'll appreciate a heads up.
DadaDaMotha: @nphilipp I went thorugh the changes and I can also not spot a difference between `GermanLocal` and `SwissLocale`. Thanks for pinging us!
nphilipp: @href @DadaDaMotha, the latest push reintroduces `SwissLocale` as its own shim (just as `GermanLocale`) around `GermanBaseLocale` for `de_ch` to not break consumers using the class directly.

Additionally, it adds a test for `locales.get_locale_by_class_name()` which was recently introduced to get coverage back up.

Patch:
diff --git a/arrow/locales.py b/arrow/locales.py
index c62d037..36e07f9 100644
--- a/arrow/locales.py
+++ b/arrow/locales.py
@@ -29,7 +29,7 @@ def get_locale_by_class_name(name):
     :param name: the name of the locale class.
 
     """
-    locale_cls = globals()[name]
+    locale_cls = globals().get(name)
 
     if locale_cls is None:
         raise ValueError("Unsupported locale '{}'".format(name))
@@ -1242,20 +1242,24 @@ class PolishLocale(SlavicBaseLocale):
     past = "{0} temu"
     future = "za {0}"
 
+    # The nouns should be in genitive case (Polish: "dopełniacz")
+    # in order to correctly form `past` & `future` expressions.
     timeframes = {
         "now": "teraz",
-        "second": "sekunda",
-        "seconds": "{0} kilka sekund",
+        "second": "sekundę",
+        "seconds": ["{0} sekund", "{0} sekundy", "{0} sekund"],
         "minute": "minutę",
         "minutes": ["{0} minut", "{0} minuty", "{0} minut"],
-        "hour": "godzina",
+        "hour": "godzinę",
         "hours": ["{0} godzin", "{0} godziny", "{0} godzin"],
         "day": "dzień",
-        "days": ["{0} dzień", "{0} dni", "{0} dni"],
+        "days": "{0} dni",
+        "week": "tydzień",
+        "weeks": ["{0} tygodni", "{0} tygodnie", "{0} tygodni"],
         "month": "miesiąc",
-        "months": ["{0} miesiąc", "{0} miesiące", "{0} miesięcy"],
+        "months": ["{0} miesięcy", "{0} miesiące", "{0} miesięcy"],
         "year": "rok",
-        "years": ["{0} rok", "{0} lata", "{0} lat"],
+        "years": ["{0} lat", "{0} lata", "{0} lat"],
     }
 
     month_names = [
@@ -1649,7 +1653,7 @@ class MacedonianLocale(SlavicBaseLocale):
     ]
 
 
-class DeutschBaseLocale(Locale):
+class GermanBaseLocale(Locale):
 
     past = "vor {0}"
     future = "in {0}"
@@ -1665,6 +1669,8 @@ class DeutschBaseLocale(Locale):
         "hours": "{0} Stunden",
         "day": "einem Tag",
         "days": "{0} Tagen",
+        "week": "einer Woche",
+        "weeks": "{0} Wochen",
         "month": "einem Monat",
         "months": "{0} Monaten",
         "year": "einem Jahr",
@@ -1675,6 +1681,7 @@ class DeutschBaseLocale(Locale):
     timeframes_only_distance["minute"] = "eine Minute"
     timeframes_only_distance["hour"] = "eine Stunde"
     timeframes_only_distance["day"] = "ein Tag"
+    timeframes_only_distance["week"] = "eine Woche"
     timeframes_only_distance["month"] = "ein Monat"
     timeframes_only_distance["year"] = "ein Jahr"
 
@@ -1734,21 +1741,28 @@ class DeutschBaseLocale(Locale):
         :param only_distance: return only distance eg: "11 seconds" without "in" or "ago" keywords
         """
 
-        humanized = self.timeframes_only_distance[timeframe].format(trunc(abs(delta)))
-
         if not only_distance:
-            humanized = self._format_timeframe(timeframe, delta)
-            humanized = self._format_relative(humanized, timeframe, delta)
+            return super(GermanBaseLocale, self).describe(
+                timeframe, delta, only_distance
+            )
+
+        # German uses a different case without 'in' or 'ago'
+        humanized = self.timeframes_only_distance[timeframe].format(trunc(abs(delta)))
 
         return humanized
 
 
-class GermanLocale(DeutschBaseLocale, Locale):
+class GermanLocale(GermanBaseLocale, Locale):
 
     names = ["de", "de_de"]
 
 
-class AustrianLocale(DeutschBaseLocale, Locale):
+class SwissLocale(GermanBaseLocale, Locale):
+
+    names = ["de_ch"]
+
+
+class AustrianLocale(GermanBaseLocale, Locale):
 
     names = ["de_at"]
 
@@ -3765,77 +3779,6 @@ class RomanshLocale(Locale):
     day_abbreviations = ["", "gli", "ma", "me", "gie", "ve", "so", "du"]
 
 
-class SwissLocale(Locale):
-
-    names = ["de", "de_ch"]
-
-    past = "vor {0}"
-    future = "in {0}"
-
-    timeframes = {
-        "now": "gerade eben",
-        "second": "eine Sekunde",
-        "seconds": "{0} Sekunden",
-        "minute": "einer Minute",
-        "minutes": "{0} Minuten",
-        "hour": "einer Stunde",
-        "hours": "{0} Stunden",
-        "day": "einem Tag",
-        "days": "{0} Tagen",
-        "week": "einer Woche",
-        "weeks": "{0} Wochen",
-        "month": "einem Monat",
-        "months": "{0} Monaten",
-        "year": "einem Jahr",
-        "years": "{0} Jahren",
-    }
-
-    month_names = [
-        "",
-        "Januar",
-        "Februar",
-        "März",
-        "April",
-        "Mai",
-        "Juni",
-        "Juli",
-        "August",
-        "September",
-        "Oktober",
-        "November",
-        "Dezember",
-    ]
-
-    month_abbreviations = [
-        "",
-        "Jan",
-        "Feb",
-        "Mär",
-        "Apr",
-        "Mai",
-        "Jun",
-        "Jul",
-        "Aug",
-        "Sep",
-        "Okt",
-        "Nov",
-        "Dez",
-    ]
-
-    day_names = [
-        "",
-        "Montag",
-        "Dienstag",
-        "Mittwoch",
-        "Donnerstag",
-        "Freitag",
-        "Samstag",
-        "Sonntag",
-    ]
-
-    day_abbreviations = ["", "Mo", "Di", "Mi", "Do", "Fr", "Sa", "So"]
-
-
 class RomanianLocale(Locale):
     names = ["ro", "ro_ro"]
 


Test patch:
diff --git a/tests/test_locales.py b/tests/test_locales.py
index 99889e3..0125763 100644
--- a/tests/test_locales.py
+++ b/tests/test_locales.py
@@ -53,6 +53,22 @@ class TestModule:
 
         assert result == mock_locale
 
+    def test_get_locale_by_class_name(self, mocker):
+        mock_locale_cls = mocker.Mock()
+        mock_locale_obj = mock_locale_cls.return_value = mocker.Mock()
+
+        globals_fn = mocker.Mock()
+        globals_fn.return_value = {"NonExistentLocale": mock_locale_cls}
+
+        with pytest.raises(ValueError):
+            arrow.locales.get_locale_by_class_name("NonExistentLocale")
+
+        mocker.patch.object(locales, "globals", globals_fn)
+        result = arrow.locales.get_locale_by_class_name("NonExistentLocale")
+
+        mock_locale_cls.assert_called_once_with()
+        assert result == mock_locale_obj
+
     def test_locales(self):
 
         assert len(locales._locales) > 0
@@ -216,15 +232,55 @@ class TestRussianLocale:
 @pytest.mark.usefixtures("lang_locale")
 class TestPolishLocale:
     def test_plurals(self):
+
+        assert self.locale._format_timeframe("seconds", 0) == "0 sekund"
+        assert self.locale._format_timeframe("second", 1) == "sekundę"
+        assert self.locale._format_timeframe("seconds", 2) == "2 sekundy"
+        assert self.locale._format_timeframe("seconds", 5) == "5 sekund"
+        assert self.locale._format_timeframe("seconds", 21) == "21 sekund"
+        assert self.locale._format_timeframe("seconds", 22) == "22 sekundy"
+        assert self.locale._format_timeframe("seconds", 25) == "25 sekund"
+
+        assert self.locale._format_timeframe("minutes", 0) == "0 minut"
+        assert self.locale._format_timeframe("minute", 1) == "minutę"
+        assert self.locale._format_timeframe("minutes", 2) == "2 minuty"
+        assert self.locale._format_timeframe("minutes", 5) == "5 minut"
+        assert self.locale._format_timeframe("minutes", 21) == "21 minut"
+        assert self.locale._format_timeframe("minutes", 22) == "22 minuty"
+        assert self.locale._format_timeframe("minutes", 25) == "25 minut"
+
         assert self.locale._format_timeframe("hours", 0) == "0 godzin"
-        assert self.locale._format_timeframe("hours", 1) == "1 godzin"
+        assert self.locale._format_timeframe("hour", 1) == "godzinę"
         assert self.locale._format_timeframe("hours", 2) == "2 godziny"
-        assert self.locale._format_timeframe("hours", 4) == "4 godziny"
         assert self.locale._format_timeframe("hours", 5) == "5 godzin"
         assert self.locale._format_timeframe("hours", 21) == "21 godzin"
         assert self.locale._format_timeframe("hours", 22) == "22 godziny"
         assert self.locale._format_timeframe("hours", 25) == "25 godzin"
 
+        assert self.locale._format_timeframe("weeks", 0) == "0 tygodni"
+        assert self.locale._format_timeframe("week", 1) == "tydzień"
+        assert self.locale._format_timeframe("weeks", 2) == "2 tygodnie"
+        assert self.locale._format_timeframe("weeks", 5) == "5 tygodni"
+        assert self.locale._format_timeframe("weeks", 21) == "21 tygodni"
+        assert self.locale._format_timeframe("weeks", 22) == "22 tygodnie"
+        assert self.locale._format_timeframe("weeks", 25) == "25 tygodni"
+
+        assert self.locale._format_timeframe("months", 0) == "0 miesięcy"
+        assert self.locale._format_timeframe("month", 1) == "miesiąc"
+        assert self.locale._format_timeframe("months", 2) == "2 miesiące"
+        assert self.locale._format_timeframe("months", 5) == "5 miesięcy"
+        assert self.locale._format_timeframe("months", 21) == "21 miesięcy"
+        assert self.locale._format_timeframe("months", 22) == "22 miesiące"
+        assert self.locale._format_timeframe("months", 25) == "25 miesięcy"
+
+        assert self.locale._format_timeframe("years", 0) == "0 lat"
+        assert self.locale._format_timeframe("year", 1) == "rok"
+        assert self.locale._format_timeframe("years", 2) == "2 lata"
+        assert self.locale._format_timeframe("years", 5) == "5 lat"
+        assert self.locale._format_timeframe("years", 21) == "21 lat"
+        assert self.locale._format_timeframe("years", 22) == "22 lata"
+        assert self.locale._format_timeframe("years", 25) == "25 lat"
+
 
 @pytest.mark.usefixtures("lang_locale")
 class TestIcelandicLocale:
@@ -466,11 +522,18 @@ class TestGermanLocale:
         assert self.locale.describe("hour", only_distance=False) == "in einer Stunde"
         assert self.locale.describe("day", only_distance=True) == "ein Tag"
         assert self.locale.describe("day", only_distance=False) == "in einem Tag"
+        assert self.locale.describe("week", only_distance=True) == "eine Woche"
+        assert self.locale.describe("week", only_distance=False) == "in einer Woche"
         assert self.locale.describe("month", only_distance=True) == "ein Monat"
         assert self.locale.describe("month", only_distance=False) == "in einem Monat"
         assert self.locale.describe("year", only_distance=True) == "ein Jahr"
         assert self.locale.describe("year", only_distance=False) == "in einem Jahr"
 
+    def test_weekday(self):
+        dt = arrow.Arrow(2015, 4, 11, 17, 30, 00)
+        assert self.locale.day_name(dt.isoweekday()) == "Samstag"
+        assert self.locale.day_abbreviation(dt.isoweekday()) == "Sa"
+
 
 @pytest.mark.usefixtures("lang_locale")
 class TestHungarianLocale:
@@ -533,16 +596,6 @@ class TestBengaliLocale:
         assert self.locale._ordinal_number(-1) is None
 
 
-@pytest.mark.usefixtures("lang_locale")
-class TestSwissLocale:
-    def test_ordinal_number(self):
-        dt = arrow.Arrow(2015, 4, 11, 17, 30, 00)
-
-        assert self.locale._format_timeframe("minute", 1) == "einer Minute"
-        assert self.locale._format_timeframe("hour", 1) == "einer Stunde"
-        assert self.locale.day_abbreviation(dt.isoweekday()) == "Sa"
-
-
 @pytest.mark.usefixtures("lang_locale")
 class TestRomanianLocale:
     def test_timeframes(self):