Skip to content

Stop parsing strings with only skipped words as today - #1440

Merged
AdrianAtZyte merged 6 commits into
scrapinghub:masterfrom
AdrianAtZyte:reject-dateless-strings
Oct 27, 2026
Merged

AdrianAtZyte merged 6 commits into
scrapinghub:masterfrom
AdrianAtZyte:reject-dateless-strings

Conversation

@AdrianAtZyte

@AdrianAtZyte AdrianAtZyte commented Sep 24, 2026 •

Copy link
Copy Markdown
Member

Fixes #1020, fixes #1032

@codspeed

codspeed Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 10 untouched benchmarks


Comparing AdrianAtZyte:reject-dateless-strings (7612ad0) with master (699855a)

Open in CodSpeed

@codecov

codecov Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.18%. Comparing base (699855a) to head (7612ad0).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1440   +/-   ##
=======================================
  Coverage   99.18%   99.18%           
=======================================
  Files         239      239           
  Lines        4054     4058    +4     
=======================================
+ Hits         4021     4025    +4     
  Misses         33       33           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@AdrianAtZyte
AdrianAtZyte marked this pull request as ready for review September 24, 2026 10:32

@serhii73 serhii73 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review (claude code-review, at e198d57). The full suite passes on this head (24612 passed) and I found no case that was right on master and is wrong here; the strings it changes ('year', 'hour minute', 't') returned today's date before.

Non-blocking notes:

  • Because the skipped-word-only parse is now rejected in the parser, the language loop falls through to the next locale, so a single letter can produce a different spurious date: parse('h', settings={'RELATIVE_BASE': datetime(2020,5,5,10)}) gave 2020-05-05 (en) on master and now gives 2020-05-04 via Hungarian h (Monday). Both are wrong, but a check in the applicability or skip-word layer would not change which locale wins.
  • all() over an empty filtered_tokens is True, so an empty token list now raises "Nothing to parse" too; please note it or add a test.
  • The check compares token strings and ignores token_type, and duplicates the skip logic of the loop below it; a single pass counting non-skipped tokens would be simpler.
  • Tests cover year, hour minute, t and one search sentence. The docs text about PARSERS=[custom-formats] with date_formats and the single-letter behaviour across locales have no test.

Keep both sentences of the date_formats docstring paragraphs: the PARSERS
fallback sentence of this PR and the %z/%Z sentence from scrapinghub#1424.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@serhii73

Copy link
Copy Markdown
Collaborator

I guess we can feel free to merge this PR, @AdrianAtZyte

@AdrianAtZyte
AdrianAtZyte merged commit 8ed5f3a into scrapinghub:master Oct 27, 2026
19 checks passed
serhii73 added a commit that referenced this pull request Nov 4, 2026
#1564)

Regression from #1401 and #1440. search_dates found the date that a relative
date is relative to by looking back through the items parsed so far for the
last one that is a date. Since #1401 the look-back goes over every item that
has no date, for example a word like "day" or "ngày" that does not parse as a
date on its own ("hour" is one too since #1440), so in a long run of such
words each word looked back over all the words before it, and each word also
copied the whole list of items parsed so far.

Keep that date up to date as each item is parsed instead, and hand it to
parse_item in place of the list. The date is the same, so the results do not
change, but the time spent on a word no longer depends on how many words came
before it.

The new tests are in their own file, tests/test_search_unparsed_word_runs.py.
Two of them count how many times a DateData is read for runs of 200 and 400
words: the count doubles with this change and almost quadruples without it
(they fail on master and pass here). Five pin which date each relative date is
relative to (they pass on both).

Results are unchanged: a differential run of master against this change, with
a frozen clock, over 5,156 strings (26,929 comparisons of parse,
get_date_data, search_dates and parse_found_objects) showed no difference.

Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Automatic search return incorrect result parse parsing strings not matching date_formats

2 participants