Implement HolidayBase::get_closest_holiday functionality by Rosi2143 · Pull Request #2211 · vacanza/holidays

Rosi2143 · 2025-01-04T13:18:36Z

Proposed change

a new function get_next_holiday is added to retrieve the date of the next known holiday.
Also the name of the holiday is returned.

It is possible to search forward and backward in time.

This should solve #1825

Documentation will be added once the basic changes are approved.

New country/market holidays support (thank you!)
Supported country/market holidays update (calendar discrepancy fix, localization)
Existing code/documentation/test/process quality improvement (best practice, cleanup, refactoring, optimization)
Dependency update (version deprecation/pin/upgrade)
Bugfix (non-breaking change which fixes an issue)
Breaking change (a code change causing existing functionality to break)
New feature (new holidays functionality in general)

Checklist

I've followed the [contributing guidelines][contributing-guidelines]
I've successfully run make check, all checks and tests are green

codecov · 2025-01-04T15:08:45Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (4d07367) to head (000d71e).
Report is 1 commits behind head on dev.

Additional details and impacted files

@@            Coverage Diff            @@
##               dev     #2211   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          200       200           
  Lines        12215     12230   +15     
  Branches      1737      1741    +4     
=========================================
+ Hits         12215     12230   +15

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

KJhellico · 2025-01-04T16:33:00Z

@Rosi2143, it's good start! But some checks are required. Try to run this:

from holidays.countries.ukraine import UA
ua = UA()
ua.get_next_holiday("1991-01-01", True)
ua.get_next_holiday("2022-03-08")

Rosi2143 · 2025-01-04T19:39:58Z

@KJhellico: Thanks - I didn't know that there is a start/end-date for a calender. But I guess for some countries it makes sense.

Added the checks and the documentation with the latest version.

KJhellico

@Rosi2143, , well done! Please take a look at a few suggestions.

KJhellico

That's a great speed improvement! 👍

KJhellico · 2025-01-11T15:47:00Z

Since the holidays in the holidays entity object are not always placed in chronological order, it's necessary to sort them before searching:

>>> from holidays.countries.united_states import US
>>> h = US(years=2024)
>>> h.get_next_holiday("2024-02-01")
(datetime.date(2024, 5, 27), 'Memorial Day')
>>> for d, name in h.items():
>>>     print(d, name)
2024-01-01 New Year's Day
2024-05-27 Memorial Day
2024-06-19 Juneteenth National Independence Day
2024-07-04 Independence Day
2024-09-02 Labor Day
2024-11-11 Veterans Day
2024-11-28 Thanksgiving
2024-12-25 Christmas Day
2024-01-15 Martin Luther King Jr. Day
2024-02-19 Washington's Birthday
2024-10-14 Columbus Day

Rosi2143 · 2025-01-12T11:47:02Z

Thanks for the hint that no sorting is applied to calendars.

That was one of the reasons I choose my initial implementation. I wanted to keep all the corner cases out of this function - at the cost of having a longer runtime.
My guess was that this function will not be called frequently, so a little more execution time would not hurt.

But fixed this now as well.

arkid15r

Thanks for working on this @Rosi2143
Please consider these comments:

KJhellico

This version LGTM. For Ukraine, we can override this method individually when the time comes.

PPsyrius

LGTM

Rosi2143 · 2025-02-03T08:31:30Z

Hi @KJhellico and @arkid15r ,

will you merge this? I have no write access to your branch.

Thanks

arkid15r · 2025-02-05T19:18:06Z

Hi @KJhellico and @arkid15r ,

will you merge this? I have no write access to your branch.

Thanks

I'll review it this week. Sorry for the delay.

a new function get_next_holiday is added to retrieve the date of the next known holiday. Also the name of the holiday is returned. It is possible to search forward and backward in time. This should solve vacanza#1825 Signed-off-by: Schrotti <Schrott.Micha@web.de>

add test to check that years without holiday are handled correctly Signed-off-by: Schrotti <Schrott.Micha@web.de>

Signed-off-by: Schrotti <Schrott.Micha@web.de>

arkid15r

Hi @Rosi2143
I've updated the implementation. Please let me know if you have any suggestions/comments.

KJhellico

LGTM.

PPsyrius

LGTM

sonarqubecloud · 2025-02-11T01:48:28Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Rosi2143 · 2025-02-11T19:39:29Z

Thanks 😃

arkid15r · 2025-02-11T19:58:13Z

Thanks 😃

Sorry I forgot to thank you yesterday after merging this.

@Rosi2143 thanks for your continuous contributions to the code base and for implementing this new functionality for all holidays users 🎉

Rosi2143 requested review from KJhellico, PPsyrius and arkid15r as code owners January 4, 2025 13:18

github-actions Bot added the test label Jan 4, 2025

Rosi2143 force-pushed the add_get_next_holiday branch from 053f942 to 562bfa1 Compare January 4, 2025 20:19

github-actions Bot added the doc label Jan 4, 2025

Rosi2143 force-pushed the add_get_next_holiday branch from 562bfa1 to c91e03e Compare January 4, 2025 21:07

KJhellico reviewed Jan 7, 2025

View reviewed changes

Comment thread docs/source/examples.rst Outdated

Comment thread holidays/holiday_base.py

Comment thread holidays/holiday_base.py Outdated

Rosi2143 force-pushed the add_get_next_holiday branch from b353dde to a5554fb Compare January 9, 2025 20:36

KJhellico reviewed Jan 10, 2025

View reviewed changes

Comment thread holidays/holiday_base.py Outdated

Comment thread holidays/holiday_base.py Outdated

Comment thread holidays/holiday_base.py Outdated

Comment thread holidays/holiday_base.py Outdated

Rosi2143 force-pushed the add_get_next_holiday branch from a5554fb to c9cd832 Compare January 10, 2025 20:20

arkid15r reviewed Jan 10, 2025

View reviewed changes

Comment thread tests/test_holiday_base.py Outdated

Rosi2143 force-pushed the add_get_next_holiday branch from c9cd832 to adf4653 Compare January 10, 2025 21:41

Rosi2143 force-pushed the add_get_next_holiday branch from 8934029 to e56effc Compare January 12, 2025 11:44

KJhellico reviewed Jan 12, 2025

View reviewed changes

Comment thread holidays/holiday_base.py Outdated

KJhellico reviewed Jan 13, 2025

View reviewed changes

Comment thread tests/test_holiday_base.py Outdated

arkid15r reviewed Jan 15, 2025

View reviewed changes

Comment thread holidays/holiday_base.py Outdated

Comment thread holidays/holiday_base.py Outdated

Comment thread holidays/holiday_base.py Outdated

Comment thread holidays/holiday_base.py Outdated

Rosi2143 force-pushed the add_get_next_holiday branch from 677ac23 to 6fda9a8 Compare January 16, 2025 16:55

Rosi2143 force-pushed the add_get_next_holiday branch from ec129fb to ecb340a Compare January 28, 2025 17:35

KJhellico reviewed Jan 28, 2025

View reviewed changes

PPsyrius previously approved these changes Jan 29, 2025

View reviewed changes

Rosi2143 force-pushed the add_get_next_holiday branch from 191ec86 to bc9f92e Compare January 31, 2025 14:48

Rosi2143 added 3 commits February 7, 2025 22:41

Change name of function to "get_closest_holiday"

04cb7cb

add test to check that years without holiday are handled correctly Signed-off-by: Schrotti <Schrott.Micha@web.de>

remove added function get_entries_sorted again

7b9b2b4

Signed-off-by: Schrotti <Schrott.Micha@web.de>

Fix maintainability warnings

bc39a89

Signed-off-by: Schrotti <Schrott.Micha@web.de>

Rosi2143 force-pushed the add_get_next_holiday branch from 8921633 to bc39a89 Compare February 7, 2025 21:41

arkid15r added 2 commits February 9, 2025 15:07

Merge branch 'dev' into pr/Rosi2143/2211

81391be

Update code

74697a7

arkid15r dismissed PPsyrius’s stale review via 74697a7 February 10, 2025 16:06

Merge branch 'dev' into add_get_next_holiday

5303e4e

arkid15r reviewed Feb 10, 2025

View reviewed changes

Comment thread holidays/holiday_base.py

arkid15r requested review from KJhellico and PPsyrius February 10, 2025 16:09

KJhellico reviewed Feb 10, 2025

View reviewed changes

arkid15r changed the title ~~add function to get the next holiday~~ Implement HolidayBase::get_closest_holiday functionality Feb 10, 2025

PPsyrius previously approved these changes Feb 11, 2025

View reviewed changes

arkid15r added 2 commits February 10, 2025 17:29

Merge branch 'dev' into pr/Rosi2143/2211

8c24f5f

Update docs

000d71e

arkid15r dismissed PPsyrius’s stale review via 000d71e February 11, 2025 01:47

arkid15r requested review from KJhellico and PPsyrius February 11, 2025 01:47

PPsyrius approved these changes Feb 11, 2025

View reviewed changes

arkid15r enabled auto-merge February 11, 2025 04:42

KJhellico approved these changes Feb 11, 2025

View reviewed changes

arkid15r added this pull request to the merge queue Feb 11, 2025

Merged via the queue into vacanza:dev with commit d55c5b4 Feb 11, 2025

Rosi2143 deleted the add_get_next_holiday branch February 11, 2025 19:39

KJhellico mentioned this pull request Feb 17, 2025

v0.67 #2300

Merged

Uh oh!

Conversation

Rosi2143 commented Jan 4, 2025

Proposed change

Checklist

Uh oh!

codecov Bot commented Jan 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

KJhellico commented Jan 4, 2025

Uh oh!

Rosi2143 commented Jan 4, 2025

Uh oh!

KJhellico left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

KJhellico left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

KJhellico commented Jan 11, 2025

Uh oh!

Rosi2143 commented Jan 12, 2025

Uh oh!

Uh oh!

Uh oh!

arkid15r left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

KJhellico left a comment

Choose a reason for hiding this comment

Uh oh!

PPsyrius left a comment

Choose a reason for hiding this comment

Uh oh!

Rosi2143 commented Feb 3, 2025

Uh oh!

arkid15r commented Feb 5, 2025

Uh oh!

arkid15r left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

KJhellico left a comment

Choose a reason for hiding this comment

Uh oh!

PPsyrius left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Feb 11, 2025

Quality Gate passed

Uh oh!

Rosi2143 commented Feb 11, 2025

Uh oh!

arkid15r commented Feb 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov Bot commented Jan 4, 2025 •

edited

Loading