-
Notifications
You must be signed in to change notification settings - Fork 465
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
[dateparser] Add feature add_prefer_month_year #1146
Conversation
- Add tests for this feature - #1042
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.
Looks great!
b31c4c4
to
03710f8
Compare
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.
On the testing side, I think it would be good to cover an additional case where the current month has fewer days than the requested month. For example: Input: 2023, Current: April 15th, Preferred: last day, last month, Expected: Dec 31st (i.e. we are making sure it does not cause Dec 30th).
- For example: Input: 2023, Current: April 15th, Preferred: last day, last month, Expected: Dec 31st (i.e. we are making sure it does not cause Dec 30th). - #1042
Thanks @Gallaecio for useful suggestion! I've added the suggested test. Would you please review it again? |
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.
Nice!
@Gallaecio Just checking on when are you planning to push a new release which includes above changes? |
Resolves #1042
To do: