diff --git a/ovos_date_parser/__init__.py b/ovos_date_parser/__init__.py index 04a0cb5..fad5e9b 100644 --- a/ovos_date_parser/__init__.py +++ b/ovos_date_parser/__init__.py @@ -29,6 +29,7 @@ from ovos_date_parser.scoped_en import extract_scoped_date_en, SCOPED_VOCAB_EN from ovos_date_parser.holidays import (extract_holiday_date, extract_holiday_span, + holiday_overrides_engine, holiday_surfaces) from ovos_date_parser.eras_en import extract_era_date_en, ERA_PATTERNS_EN from ovos_date_parser.eras_pt import extract_era_date_pt, ERA_PATTERNS_PT @@ -412,15 +413,28 @@ def extract_datetime( reads no date, :func:`ovos_date_parser.holidays.extract_holiday_span` asks chronologia, and its answer is taken only when chronologia says a holiday construction is what matched. An utterance the engine already - reads keeps the engine's answer, so nothing that worked before changes. + reads keeps the engine's answer, with one exception: a holiday whose own + name is written with calendar vocabulary ("good friday", "palm sunday") + is read by the engine as that weekday, so when the engine answers the + holiday phrase on its own with the same date it gave the whole text + (:func:`~ovos_date_parser.holidays.holiday_overrides_engine`) the holiday + layer answers instead. A time of day, an ordinal or any other word beside + the holiday name does not stop this, because it does not move the date. + A date the engine read from a calendar word the phrase does not cover is + a different date, and stays the engine's. """ found = _extract_datetime_engine(text, lang, anchorDate=anchorDate, default_time=default_time) - if found is not None: + if found is not None and not holiday_overrides_engine( + text, lang, found[0], found[1], anchorDate=anchorDate): return found holiday = extract_holiday_span(text, lang, anchorDate=anchorDate) if holiday is None: - return None + # one return for both ways of arriving here: the engine read nothing + # and ``found`` is None, which is the old contract, or the override + # sent an engine answer to a holiday layer that then declined, and + # the engine's answer stands. + return found moment, remainder = holiday if default_time is not None: moment = moment.replace(hour=default_time.hour, diff --git a/ovos_date_parser/holidays.py b/ovos_date_parser/holidays.py index 6e6b51b..3595648 100644 --- a/ovos_date_parser/holidays.py +++ b/ovos_date_parser/holidays.py @@ -334,6 +334,103 @@ def extract_holiday_span(text: str, lang: str, return start, remainder.strip() +def holiday_overrides_engine(text: str, lang: str, engine_date: datetime, + engine_remainder: str = "", + anchorDate: Optional[datetime] = None) -> bool: + """Whether a language engine read its date out of the holiday phrase. + + A holiday name may be written with calendar vocabulary of its own + language: "good friday", "palm sunday", "may day". A per-language engine + reads the weekday inside such a name and answers with the coming Friday, + and the holiday layer, asked only when the engine found nothing, is never + reached. The engine order is right everywhere else, so the question asked + here is narrow: did the engine take its date from the holiday phrase + itself? + + Args: + text: the text the engine was given. + lang: the BCP-47 code the engine was given. + engine_date: the datetime the engine answered with. Its date is the + value compared; its time is not read. + engine_remainder: what the engine left over. Read only by the cheap + guard below, which declines without a second engine call when the + engine consumed no word of the holiday phrase. + anchorDate: the date relative dating is reckoned from. + + The question is answered by asking the engine the holiday phrase on its + own. The override fires when that answer's date equals ``engine_date``, + the date the engine gave the whole text. For "good friday" the slice + "good friday" gives the coming Friday and so does the whole text, so the + engine read its date out of the phrase. For "play christmas music on + friday" the slice is "christmas", which the engine cannot read at all, so + the Friday came from elsewhere and stays the engine's. For "good friday + and next friday" the slice gives the coming Friday and the whole text + gives the one after, so the dates differ and the engine keeps its answer. + + Only the date is compared, never the time. A time of day beside the + holiday name does not move the date, so "good friday at 9am" and + "remind me on good friday at 9am" override exactly as the bare name does. + That is this function's reason for existing in this shape: the first cut + compared the words the engine consumed against the words of the phrase + and declined whenever the engine consumed anything else, which left half + the holiday's name in the remainder on the commonest spoken form of all + eight of these holidays. + + The holiday layer answers with a date, so the engine's time of day is not + carried onto it. "good friday at 9am" gives Good Friday at midnight and + leaves "at 9am" in the remainder. + + A date word that moves the date keeps the engine's answer, which is the + right outcome for "good friday and next friday" and the wrong one for + "the friday before good friday": the slice and the whole text both give + the coming Friday there, so the override fires and the answer is Good + Friday rather than the Friday before it. "monday after easter monday" + is the same shape. Both were already wrong before this function existed, + where the engine gave the coming weekday, so this is a bound on the fix + and not a regression. To decide it properly, compare character offsets + against the extent ``(first, last)`` computed below. + ``test_holidays_chronologia.py`` carries a cell for each known answer, so + the day the extent becomes positional the cell says what changed. + + The engine is asked twice for a text that names a holiday of its + language, once by the caller and once here, so such a text costs more + than one that names none. A text that names no holiday pays nothing: the + table lookup below answers first. + + A text that names no holiday of its language never reaches chronologia: + :func:`_names_a_holiday` is a table lookup and answers first. + """ + if not text: + return False + if not _names_a_holiday(text, lang): + return False + anchor = anchorDate or datetime.now() + written = _as_written(text, lang) + extent = _holiday_extent(written, lang, anchor) + if extent is None: + return False + first, last = extent + phrase_words = _word_set(written[first:last]) + consumed = _word_set(text) - _word_set(engine_remainder) + if not (consumed & phrase_words): + # The engine read its date from words the phrase does not cover, so + # the slice below cannot give the same date except by coincidence. + # Declining here is what keeps a text that merely names a holiday + # ("play christmas music on friday") to one engine call. + return False + # imported here, not at module scope: ovos_date_parser/__init__.py imports + # this module, so a top-level import would be a cycle. + from ovos_date_parser import _extract_datetime_engine + inner = _extract_datetime_engine(written[first:last], lang, + anchorDate=anchor) + return inner is not None and inner[0].date() == engine_date.date() + + +def _word_set(text: str) -> FrozenSet[str]: + """The folded words of ``text``, for comparing one extent with another.""" + return frozenset(_fold(word) for word in re.findall(r"\w+", text or "")) + + def extract_holiday_date(text: str, lang: str, ref_date: Optional[date] = None ) -> Optional[Tuple[date, str]]: diff --git a/test/test_holidays_chronologia.py b/test/test_holidays_chronologia.py index 8a873cc..c12c212 100644 --- a/test/test_holidays_chronologia.py +++ b/test/test_holidays_chronologia.py @@ -8,7 +8,7 @@ The anchor is fixed at 25 September 2026, so "the next one" is a stated date rather than whatever today makes it. """ -from datetime import date, datetime +from datetime import date, datetime, time import pytest @@ -227,3 +227,199 @@ def test_a_tense_inside_the_holiday_phrase_still_reads(utterance, expected): "christmas eve" each as one match over all their words. """ assert extract_datetime(utterance, "en-US", REF)[0] == expected + + +# --- finding 5 of the #369 review: a holiday named after a weekday ---------- + +# `extract_datetime` runs the per-language engine first. A holiday whose own +# name carries weekday vocabulary — "good friday", "palm sunday" — therefore +# answered with the coming weekday and never reached the holiday layer, which +# knew the right date all along. Eight of the twelve weekday- and month-named +# English holidays answered that way. +# +# Every date below is reckoned independently of the parser. Western Easter 2027 +# is 28 March by the Gregorian computus, and the movable feasts are counted from +# it: Palm Sunday is Easter minus 7 (21 March), Maundy Thursday minus 3 +# (25 March), Good Friday minus 2 (26 March), Holy Saturday minus 1 (27 March), +# Easter Monday plus 1 (29 March), Whit Monday plus 50 (17 May). Shrove Tuesday +# is Easter minus 47 (9 February) and Ash Wednesday minus 46 (10 February). + +WEEKDAY_NAMED_HOLIDAYS = [ + ("palm sunday", date(2027, 3, 21)), + ("maundy thursday", date(2027, 3, 25)), + ("good friday", date(2027, 3, 26)), + ("holy saturday", date(2027, 3, 27)), + ("easter monday", date(2027, 3, 29)), + ("whit monday", date(2027, 5, 17)), + ("shrove tuesday", date(2027, 2, 9)), + ("ash wednesday", date(2027, 2, 10)), +] + + +@pytest.mark.parametrize("utterance,expected", WEEKDAY_NAMED_HOLIDAYS) +def test_a_weekday_named_holiday_answers_with_the_holiday(utterance, expected): + """The holiday layer's date wins when the engine read the holiday's own + words as a weekday.""" + got = extract_datetime(utterance, "en-US", REF) + assert got is not None + assert got[0].date() == expected + + +@pytest.mark.parametrize("utterance,expected", WEEKDAY_NAMED_HOLIDAYS) +def test_a_weekday_named_holiday_keeps_no_half_of_its_name(utterance, expected): + """The remainder holds no word of the holiday's own name.""" + got = extract_datetime(utterance, "en-US", REF) + assert got is not None + for word in utterance.split(): + assert word not in got[1].lower() + + +# --- a word of time beside the holiday name, from the #374 review ---------- + +# The first cut compared the words the engine consumed against the words of the +# holiday phrase, and declined whenever the engine consumed anything else. A +# time of day is something else, so "good friday at 9am" kept the engine's +# coming Friday and left `'good'` in the remainder: half the holiday's own name, +# on the commonest spoken form of all eight. The test asks the engine the +# holiday phrase on its own instead, and the override fires when that answer's +# date equals the date the whole text produced, which a time of day does not +# move. +# +# Each expected date is reckoned independently from the computus, as above. + +WEEKDAY_NAMED_HOLIDAYS_WITH_A_TIME = [ + ("good friday at 9am", date(2027, 3, 26)), + ("remind me on good friday at 9am", date(2027, 3, 26)), + ("palm sunday at noon", date(2027, 3, 21)), + ("easter monday morning", date(2027, 3, 29)), + ("shrove tuesday at 6pm", date(2027, 2, 9)), + ("set an alarm for good friday at 7", date(2027, 3, 26)), +] + +WEEKDAY_NAMED_HOLIDAY_WORDS = frozenset( + word for utterance, _ in WEEKDAY_NAMED_HOLIDAYS for word in utterance.split() +) + + +@pytest.mark.parametrize("utterance,expected", + WEEKDAY_NAMED_HOLIDAYS_WITH_A_TIME) +def test_a_time_beside_a_holiday_name_still_answers_with_the_holiday( + utterance, expected): + """A time of day beside the holiday name does not send the answer back to + the engine's weekday.""" + got = extract_datetime(utterance, "en-US", REF) + assert got is not None + assert got[0].date() == expected + + +@pytest.mark.parametrize("utterance,expected", + WEEKDAY_NAMED_HOLIDAYS_WITH_A_TIME) +def test_a_time_beside_a_holiday_name_keeps_no_half_of_the_name( + utterance, expected): + """The remainder holds no word of any weekday-named holiday. + + The whole set is checked, not only the words of this utterance, because the + failure this cell exists for left `'good'`, `'palm'`, `'easter'` and + `'shrove'` behind. + """ + got = extract_datetime(utterance, "en-US", REF) + assert got is not None + remainder_words = set(got[1].lower().split()) + assert not (remainder_words & WEEKDAY_NAMED_HOLIDAY_WORDS) + + +def test_a_time_beside_a_holiday_name_is_left_in_the_remainder(): + """Known answer, and the bound on this round: the holiday layer answers + with a date, so the engine's time of day is not carried onto it. + + "good friday at 9am" gives Good Friday at midnight and leaves "at 9am" in + the remainder. The time is not lost, but a caller that reads only the + datetime sees midnight. Carrying it would also have to take the time words + out of the remainder, which is a second change this round does not make. + The day the time is carried this cell fails and says so. + """ + got = extract_datetime("good friday at 9am", "en-US", REF) + assert got is not None + assert got[0].date() == date(2027, 3, 26) + assert got[0].time() == time(0, 0) + assert "9am" in got[1] + + +def test_a_plain_weekday_still_answers_with_the_weekday(utterance=None): + """Control: a weekday that names no holiday is untouched. + + 25 September 2026 is a Friday, so the coming Sunday is the 27th. + """ + got = extract_datetime("sunday", "en-US", REF) + assert got is not None + assert got[0].date() == date(2026, 9, 27) + + +def test_a_weekday_beside_a_holiday_keeps_the_weekday(): + """Control: the engine's answer stands when it read words the holiday + phrase does not cover. + + "play christmas music on friday" asks about Friday. The holiday phrase is + "christmas" alone, so the engine's Friday is not inside it and the holiday + date must not replace it. + """ + got = extract_datetime("play christmas music on friday", "en-US", REF) + assert got is not None + assert got[0].date() == date(2026, 9, 25) or got[0].date() == date(2026, 10, 2) + assert got[0].date() != date(2026, 12, 25) + + +def test_a_holiday_the_engine_cannot_read_is_unchanged(): + """Control: the None path of the engine still answers from the layer.""" + got = extract_datetime("boxing day", "en-US", REF) + assert got is not None + assert got[0].date() == date(2026, 12, 26) + + +# --- the bound on the extent test, from the #374 review --------------------- + +# `holiday_overrides_engine` compares a set of folded words, not a position, so +# a weekday word that appears both inside the holiday name and elsewhere in the +# sentence is indistinguishable from one that appears inside the name alone. A +# relative phrase built on such a word answers with the holiday itself. +# +# These cells record the answer this library gives today, not the answer the +# phrase asks for. Each expected date is reckoned independently: Western Easter +# 2027 is 28 March by the Gregorian computus, so Good Friday is 26 March and +# Easter Monday is 29 March. The date the phrase actually names is written +# beside each cell. The day the extent test becomes positional these cells fail +# and say what changed. + +KNOWN_ANSWER_RELATIVE_PHRASES = [ + # utterance, the date given today, the date the phrase names + ("the friday before good friday", date(2027, 3, 26), date(2027, 3, 19)), + ("monday after easter monday", date(2027, 3, 29), date(2027, 4, 5)), +] + + +@pytest.mark.parametrize("utterance,given,asked", + KNOWN_ANSWER_RELATIVE_PHRASES) +def test_a_relative_phrase_on_a_holiday_word_answers_with_the_holiday( + utterance, given, asked): + """Known answer: the holiday's own date, not the day the phrase names. + + The engine gave the coming weekday for these phrases before this fix + existed, which was wrong too, so the cell bounds the fix rather than + approving it. + """ + got = extract_datetime(utterance, "en-US", REF) + assert got is not None + assert got[0].date() == given + assert got[0].date() != asked + + +def test_a_relative_phrase_the_engine_keeps_is_not_taken(): + """Control, the opposite call on a phrase of the same shape. + + "good friday and next friday" carries a Friday outside the holiday phrase, + so the consumed words are not a subset of the phrase and the engine keeps + the utterance. 25 September 2026 is a Friday, so the next one is 2 October. + """ + got = extract_datetime("good friday and next friday", "en-US", REF) + assert got is not None + assert got[0].date() == date(2026, 10, 2)