Skip to content

Parse calendar quarters - #1417

Merged
AdrianAtZyte merged 5 commits into
scrapinghub:masterfrom
AdrianAtZyte:quarters
Oct 22, 2026
Merged

AdrianAtZyte merged 5 commits into
scrapinghub:masterfrom
AdrianAtZyte:quarters

Conversation

@AdrianAtZyte

Copy link
Copy Markdown
Member

Resolves #1254

@codspeed

codspeed Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Merging this PR will not alter performance

βœ… 10 untouched benchmarks


Comparing AdrianAtZyte:quarters (51bd6e5) with master (bd73ab7)

Open in CodSpeed

@codecov

codecov Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Codecov Report

βœ… All modified and coverable lines are covered by tests.
βœ… Project coverage is 99.04%. Comparing base (bd73ab7) to head (51bd6e5).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1417   +/-   ##
=======================================
  Coverage   99.03%   99.04%           
=======================================
  Files         239      239           
  Lines        3853     3868   +15     
=======================================
+ Hits         3816     3831   +15     
  Misses         37       37           

β˜” 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 23, 2026 11:31

@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 bcd52be; the first four items were run on this head). Standalone quarters work and I found no regression for inputs that parsed before. The main follow-up is to anchor the rewrite.

Non-blocking notes:

  • The quarter regex only requires no word character on either side, and the substitution runs on the whole string. parse('1 Q2 2025') returns 2025-01-04 (it becomes 1 2025-04), parse('Q2 2025 and 3') returns 2025-04-03, and Q2 1530 is read as the year 1530. All of these returned None on master. Applying the rewrite only when the quarter token is the whole date expression, and limiting the year to a plausible range, would avoid silently wrong dates.
  • The rewrite runs before locale selection, so parse('2025Q2', languages=['ru']) still works and every get_date_data call pays an extra substitution; a cheap 'q' in date_string.lower() guard would help.
  • "Current" for PREFER_MONTH_OF_YEAR takes now from RELATIVE_BASE or datetime.now() and ignores TIMEZONE, unlike the rest of the parser.
  • period='quarter' is only patched onto the final DateData, while _is_valid_date_data and get_intersecting_periods don't know it, and parts still reports ('year','month'). A string with several quarters (2025Q2 2025Q3) returns None.
  • Tests cover standalone quarters and three negative cases only. Please add embedded-token cases, search_dates, languages, RETURN_TIME_SPAN and TIMEZONE.

@AdrianAtZyte
AdrianAtZyte merged commit c529f8b into scrapinghub:master Oct 22, 2026
19 checks passed
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.

Support parsing quarters of the year like 2025Q02

2 participants