-
Notifications
You must be signed in to change notification settings - Fork 479
Fix short weekday names #1214
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Fix short weekday names #1214
Conversation
Please check https://github.com/scrapinghub/dateparser/blob/master/CONTRIBUTING.rst#guidelines-for-editing-translation-data about editing language data (you edited a generated file). |
1e4c73c
to
30e2c7f
Compare
- Update en.yaml file to support two letters days of the week - #1170
Any idea about the failing test? |
- Develop method to remove_multiple_occurrences of the day(s) of the week - #1170
Could you add tests for the fix itself? (7 dates with the short weekday form that are properly parsed) I think that’s all that’s left. |
- Add tests - #1170
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1214 +/- ##
==========================================
+ Coverage 97.49% 97.50% +0.01%
==========================================
Files 234 234
Lines 2793 2806 +13
==========================================
+ Hits 2723 2736 +13
Misses 70 70 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
def then_date_was_not_parsed(self): | ||
self.assertIsNone(self.result["date_obj"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems unused now.
now = datetime(year, month, day, **time) | ||
datetime_mock = Mock(wraps=datetime) | ||
datetime_mock.utcnow = Mock(return_value=now) | ||
datetime_mock.now = Mock(return_value=now) | ||
datetime_mock.today = Mock(return_value=now) | ||
self.add_patch(patch("dateparser.date.datetime", new=datetime_mock)) | ||
now = real_datetime.datetime(year, month, day, **time) | ||
|
||
# Patch the datetime *class* in each target module | ||
class DateParserDateTime(real_datetime.datetime): | ||
@classmethod | ||
def now(cls, tz=None): | ||
return now.replace(tzinfo=tz) if tz else now | ||
|
||
@classmethod | ||
def utcnow(cls): | ||
return now | ||
|
||
@classmethod | ||
def today(cls): | ||
return now | ||
|
||
self.add_patch(patch("dateparser.date.datetime", DateParserDateTime)) | ||
self.add_patch(patch("dateparser.parser.datetime", DateParserDateTime)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you explain what this change does?
@@ -145,6 +170,7 @@ def translate(self, date_string, keep_formatting=False, settings=None): | |||
if "in" in date_string_tokens: | |||
date_string_tokens = self._clear_future_words(date_string_tokens) | |||
|
|||
self.remove_multiple_occurrences(date_string_tokens) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this do? None of the new test scenarios seem to feature multiple occurrences of a week day. What scenarios does this address? (I wonder if it could be introducing other issues)
Fixes #1170