Repository navigation
Recognize month ranges fused into a single word - #63
Merged
Merged
Conversation
A tight month-month range such as "ágúst-september" (common in
phrases like "um mánaðamótin ágúst-september") was glued into a
single WORD token by hyphenated-compound handling before date
recognition could see it, so downstream parsers never received the
month names. parse_date_and_time() now splits such a word into
DATEREL, hyphen and DATEREL, for both the ASCII hyphen and the en
dash. The check is guarded cheaply: only WORD tokens containing a
hyphen character are examined further, and the month lookup only
runs on a two-part split whose halves are not ambiguous month names
("mar-apr" and capitalized "Ágúst-..." are left alone, consistent
with single-month behavior).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
vthorsteinsson
commented
Sep 2, 2026
vthorsteinsson
left a comment
vthorsteinsson
left a comment
Member
Author
There was a problem hiding this comment.
Fix performance issues
| a = token.txt.split(hchar) | ||
| if ( | ||
| len(a) == 2 | ||
| and a[0] not in AMBIGUOUS_MONTH_NAMES |
Member
Author
There was a problem hiding this comment.
more efficient: (a0 := a[0]) not in AMBIGUOUS_MONTH_NAMES
| if ( | ||
| len(a) == 2 | ||
| and a[0] not in AMBIGUOUS_MONTH_NAMES | ||
| and a[1] not in AMBIGUOUS_MONTH_NAMES |
Member
Author
There was a problem hiding this comment.
more efficient: (a1 := a[1]) not in AMBIGUOUS_MONTH_NAMES
| and a[0] not in AMBIGUOUS_MONTH_NAMES | ||
| and a[1] not in AMBIGUOUS_MONTH_NAMES | ||
| ): | ||
| m1 = MONTHS.get(a[0].lower()) |
| and a[1] not in AMBIGUOUS_MONTH_NAMES | ||
| ): | ||
| m1 = MONTHS.get(a[0].lower()) | ||
| m2 = MONTHS.get(a[1].lower()) |
| m1 = MONTHS.get(a[0].lower()) | ||
| m2 = MONTHS.get(a[1].lower()) | ||
| if m1 is not None and m2 is not None: | ||
| first, rest = token.split(len(a[0])) |
Member
Author
There was a problem hiding this comment.
Delete the assignments to first, rest, hyphen, second - they're superfluous
| if m1 is not None and m2 is not None: | ||
| first, rest = token.split(len(a[0])) | ||
| hyphen, second = rest.split(1) | ||
| yield TOK.Daterel(first, y=0, m=m1, d=0) |
Member
Author
There was a problem hiding this comment.
a0 instead of first
| first, rest = token.split(len(a[0])) | ||
| hyphen, second = rest.split(1) | ||
| yield TOK.Daterel(first, y=0, m=m1, d=0) | ||
| yield TOK.Punctuation(hyphen) |
Member
Author
There was a problem hiding this comment.
hchar instead of hyphen
| hyphen, second = rest.split(1) | ||
| yield TOK.Daterel(first, y=0, m=m1, d=0) | ||
| yield TOK.Punctuation(hyphen) | ||
| token = TOK.Daterel(second, y=0, m=m2, d=0) |
Member
Author
There was a problem hiding this comment.
a1 instead of second
Capture the two halves with walrus assignments, reuse them for the month lookups, and construct the emitted tokens directly from the strings instead of slicing the original token. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A tight month-month range such as "ágúst-september" — common in phrases like "um mánaðamótin ágúst-september" — is glued into a single WORD token by the hyphenated-compound handling before date recognition can see it, so downstream consumers (e.g. the GreynirEngine parser) never receive the month names.
parse_date_and_time()now splits such a word into DATEREL + hyphen + DATEREL, for both the ASCII hyphen and the en dash ("júlí–ágúst"). This matches what the tokenizer already produces for the spaced and slashed variants ("ágúst - september", "ágúst/september").Performance: the guard is deliberately cheap — on the common path a WORD token costs only C-level
insubstring scans; the split, two frozenset checks and two dict lookups run only for the rare hyphen-bearing words whose halves might be months.Correctly left untouched: ambiguous month abbreviations ("mar-apr"), the capitalized-Ágúst blacklist ("Ágúst-september"), ordinary hyphenated words ("vestur-þýska"), and the "fjármála-og efnahagsráðuneyti" composite path.
Full test suite passes (151 tests, including 5 new cases).
🤖 Generated with Claude Code