Skip to content
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

South African Public Holidays Naming and Duplicates #285

Closed
surfer190 opened this issue Sep 26, 2018 · 7 comments
Closed

South African Public Holidays Naming and Duplicates #285

surfer190 opened this issue Sep 26, 2018 · 7 comments
Projects

Comments

@surfer190
Copy link

surfer190 commented Sep 26, 2018

Setup

workalendar==3.0.0
Python 3.6.4

Issue

The South Africa public holidays gives:

(datetime.date(2019, 1, 1), 'New year')
(datetime.date(2019, 3, 21), 'Human Rights Day')
(datetime.date(2019, 4, 19), 'Family Day')
(datetime.date(2019, 4, 19), 'Good Friday')
(datetime.date(2019, 4, 22), 'Easter Monday')
(datetime.date(2019, 4, 27), 'Freedom Day')
(datetime.date(2019, 5, 1), 'Workers Day')
(datetime.date(2019, 6, 16), 'Youth Day')
(datetime.date(2019, 6, 17), 'Youth Day substitute')
(datetime.date(2019, 8, 9), 'National Women Day')
(datetime.date(2019, 9, 24), 'Heritage Day')
(datetime.date(2019, 12, 16), 'Day of reconcilation')
(datetime.date(2019, 12, 25), 'Christmas Day')
(datetime.date(2019, 12, 26), 'Boxing Day')
(datetime.date(2019, 12, 26), 'Day of good will')

The problems:

  • Family Day and Good Friday are on the same day. According to my source there should be a single name called Good Friday, the duplicate Family Day should be removed
  • The is no Easter Monday anymore, it is called Family Day, this should be a rename.
  • Boxing Day and Day of Goodwill are on the same day. According to my source there should be a single name called Day of Goodwill, the duplicate Boxing Day should be removed

You can view my source of South African public holidays here

@brunobord
Copy link
Member

nice remarks!
I'm going to tackle this bug as soon as I can.

@brunobord
Copy link
Member

@surfer190
Copy link
Author

Thanks for looking at this @brunobord

I was thinking about it and wanted to know if it is meant for the package user to ensure a distinct holiday is on a specific day. Ie. if there are 2 or more on a single date, then package user need to ensure only first one is used for example. I assume the first item in the iterator will be the generic name.

I think the generic good friday, boxing day and easter monday are causing the duplicates. So perhaps we just leave it as is.

include_good_friday = True
include_easter_monday = True
include_christmas = True
include_boxing_day = True

@brunobord brunobord added this to Working in Workalendar Oct 4, 2018
brunobord added a commit that referenced this issue Oct 11, 2018
refs #285

Cleans up SouthAfrica class and tests to take into account the specs of holidays that vary over the periods.
As a side-effect, it effectively cleans the duplicates that were generated by the previous code.
@brunobord
Copy link
Member

See linked PR that implements changes. It was hard work, but it made me clean up a lot of bizarre lines of code :o)

@surfer190
Copy link
Author

Thanks @brunobord everything looking good just made a small change for one of the labels

brunobord added a commit that referenced this issue Oct 25, 2018
refs #285

Cleans up SouthAfrica class and tests to take into account the specs of holidays that vary over the periods.
As a side-effect, it effectively cleans the duplicates that were generated by the previous code.
@brunobord
Copy link
Member

closed by #286 & #287

@brunobord brunobord moved this from Working to Done in Workalendar Oct 25, 2018
@brunobord brunobord mentioned this issue Oct 25, 2018
@brunobord
Copy link
Member

FYI, I've just released the v3.1.0 of workalendar that ships all the fixes. Happy upgrading!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Workalendar
  
Done/Closed/Published
Development

No branches or pull requests

2 participants