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

Make Colombian holidays preserve historical observation #676

Merged
merged 5 commits into from
Jul 23, 2022

Conversation

bkthomps
Copy link
Contributor

@bkthomps bkthomps commented Jun 26, 2022

Colombia essentially has four types of holidays:

  1. fixed non-easter based
  2. flexible non-easter based
  3. fixed easter based
  4. flexible easter based

The easter based holidays depend on when easter occurs. The flexible holidays occur on the next Monday after the date the holiday falls on, unless it already falls on a Monday.

This change does the following:

  1. fixes non-easter based holidays not being set (ie: new year's wasn't being set for 2022: https://github.com/dr-prodigy/python-holidays/issues/674).
  2. I rigorously went through the Colombian laws and decrees about holidays and labour since 1905 (wasn't precisely codified until 1939) and went through examples of physical calendars to verify this. Proof is in the code as comments.
  3. I improved the test cases to increase confidence in the code correctness.

This branch was rebased off master as instructed

@bkthomps bkthomps changed the title Fix Colombia Make Colombian holidays preserve historical observation Jun 26, 2022
@jorgeegomez
Copy link

@bkthomps Could you please reconsider the weekends code for the six fixed Colombian holidays? I think the day should be considered a holiday even if it's Saturdy or Sunday. Lots of people work on Saturday and Sunday, and it makes a difference whether the day is a holiday or not.

The current law can be found here

@bkthomps
Copy link
Contributor Author

bkthomps commented Jul 7, 2022

@bkthomps Could you please reconsider the weekends code for the six fixed Colombian holidays? I think the day should be considered a holiday even if it's Saturdy or Sunday. Lots of people work on Saturday and Sunday, and it makes a difference whether the day is a holiday or not.

The current law can be found here

Can you elaborate on what is incorrect please? Most holidays move the the next Monday, but some are always on the day which they fall on (namely those mentioned in section 1 but not section 2).

@jorgeegomez
Copy link

I think Sundays (and even more so Saturdays) should be included in the list of holidays when they're a fixed-date holiday.

In other words, I think len(holidays.CO(years=y).items() should be 18 for any year (after 1986 at least).

It's factually incorrect to say that 2022-01-01 or 2024-12-08 are not a holiday in Colombia.

@bkthomps
Copy link
Contributor Author

bkthomps commented Jul 8, 2022

I think Sundays (and even more so Saturdays) should be included in the list of holidays when they're a fixed-date holiday.

In other words, I think len(holidays.CO(years=y).items() should be 18 for any year (after 1986 at least).

It's factually incorrect to say that 2022-01-01 or 2024-12-08 are not a holiday in Colombia.

Unless I'm missing something, this is what I've change the code to do. 2021-01-01 is considered a holiday in the test cases that I've added.

Here you can see New Year's is always set

Here you can see where I'm testing this fact

I have included test cases for 2016 to 2023 which verify every single day of the year.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 2716368539

  • 37 of 37 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.002%) to 99.04%

Totals Coverage Status
Change from base Build 2468473571: -0.002%
Covered Lines: 6413
Relevant Lines: 6427

💛 - Coveralls

@dr-prodigy dr-prodigy merged commit 7be0d1d into vacanza:beta Jul 23, 2022
@dr-prodigy
Copy link
Collaborator

Perfect work! Merged in beta, thx! 👍

@dr-prodigy dr-prodigy mentioned this pull request Aug 21, 2022
dr-prodigy added a commit that referenced this pull request Aug 21, 2022
* add honduras tests

* add honduras tests

* Add tests for many edge cases

* Add tests for many edge cases

* remove duplicate function

* v.0.15 beta init

* Typechecking (#661)

* typechecking HolidayBase, holidays.utils, miscellaneous typechecking
note: coverage to be extended to other parts of the project code

* Update Ukraine holidays.

  - Add Day of Ukrainian Statehood holiday.
  - Edit Victory Day name.
  - Fix formatting.
  - Update tests for Ukraine holidays.
  - Switch to <= and >= for consistency.
  - Rename Orthodox Easter.

* CHANGES sync

* Add Day after Thanksgiving to US - PA holidays (#675)

* CHANGES sync

* CHANGES sync, pre-commit reviews

* test_ukraine pre-commit fix

* Bump actions/setup-python from 3.1.0 to 4.1.0 (#682)

* Bump pre-commit/action from 2.0.3 to 3.0.0 (#672)

* removed py36 from toml target-version array

* Add Cuba (#678)

authored-by: bthompson <bthompson@pinterest.com>

* CHANGES sync

* Make Colombian holidays preserve historical observation (#676)

Authored-by: bthompson <bthompson@pinterest.com>

* Make Venezuela holidays preserve historical observation (#677)

Authored-by: bthompson <bthompson@pinterest.com>

* CHANGES sync

* black fixes

* removed outdated deprecation warnings

* Drop support for PortugalExt

* Ensure Canada Christmas observed falls after 25 and not before - new version #579

* CHANGES sync

* README sync (support for Cuba)

* Drop support for UK subdivisions as countries (England, Scotland..) (dr-p)
Drop support for IsleOfMan as UK subdivision (dr-p)

* Added support for Python 3.11, improved README badges

* Fixed CD/CI setup.cfg typo

* Added py3.11 to toml config

* Removed py3.11 from setup.cfg due to failing pre-commit checks

* Bump pypa/gh-action-pypi-publish from 1.5.0 to 1.5.1 #686

* Doc example fix (#685)

* added pentecost monday for province of bolzano

source: https://it.wikipedia.org/wiki/Pentecoste

* Whitsun Monday - province of Bolzano - IT (#689)

* Bump actions/setup-python to 4.2.0 (#688)

* v.0.15

Co-authored-by: Akos <afurton@apple.com>
Co-authored-by: Akos Furton <furton.akos@bcg.com>
Co-authored-by: Maurizio Montel <mmontel@mmontel.mxp.csb>
Co-authored-by: David Hotham <david.hotham@blueyonder.co.uk>
Co-authored-by: Kate <kate@kgthreads.com>
Co-authored-by: Ark <ark@cho.red>
Co-authored-by: Bailey Thompson <bailey.kyle.thompson@gmail.com>
Co-authored-by: g-gg <52978382+g-gg@users.noreply.github.com>
@bkthomps bkthomps deleted the bkthomps/fix-colombia branch August 25, 2022 03:36
dr-prodigy added a commit that referenced this pull request Sep 16, 2022
* add honduras tests

* add honduras tests

* Add tests for many edge cases

* Add tests for many edge cases

* remove duplicate function

* v.0.15 beta init

* Typechecking (#661)

* typechecking HolidayBase, holidays.utils, miscellaneous typechecking
note: coverage to be extended to other parts of the project code

* Update Ukraine holidays.

  - Add Day of Ukrainian Statehood holiday.
  - Edit Victory Day name.
  - Fix formatting.
  - Update tests for Ukraine holidays.
  - Switch to <= and >= for consistency.
  - Rename Orthodox Easter.

* CHANGES sync

* Add Day after Thanksgiving to US - PA holidays (#675)

* CHANGES sync

* CHANGES sync, pre-commit reviews

* test_ukraine pre-commit fix

* Bump actions/setup-python from 3.1.0 to 4.1.0 (#682)

* Bump pre-commit/action from 2.0.3 to 3.0.0 (#672)

* removed py36 from toml target-version array

* Add Cuba (#678)

authored-by: bthompson <bthompson@pinterest.com>

* CHANGES sync

* Make Colombian holidays preserve historical observation (#676)

Authored-by: bthompson <bthompson@pinterest.com>

* Make Venezuela holidays preserve historical observation (#677)

Authored-by: bthompson <bthompson@pinterest.com>

* CHANGES sync

* black fixes

* removed outdated deprecation warnings

* Drop support for PortugalExt

* Ensure Canada Christmas observed falls after 25 and not before - new version #579

* CHANGES sync

* README sync (support for Cuba)

* Drop support for UK subdivisions as countries (England, Scotland..) (dr-p)
Drop support for IsleOfMan as UK subdivision (dr-p)

* Added support for Python 3.11, improved README badges

* Fixed CD/CI setup.cfg typo

* Added py3.11 to toml config

* Removed py3.11 from setup.cfg due to failing pre-commit checks

* Bump pypa/gh-action-pypi-publish from 1.5.0 to 1.5.1 #686

* Doc example fix (#685)

* added pentecost monday for province of bolzano

source: https://it.wikipedia.org/wiki/Pentecoste

* Whitsun Monday - province of Bolzano - IT (#689)

* Bump actions/setup-python to 4.2.0 (#688)

* v.0.15

* v.0.16 beta init

* Add UK Holiday for Queen Elizabeth II Funeral (#702)

* Add Moldova (#695)

* Add Bolivian holidays (#698)

* CHANGES sync

* Fix NYSE New Year observance for Saturdays. (#696)

* CHANGES sync

* Financial market support review, new method financial_holidays(..) #694

* Add Australia National Day of Mourning and confirmed Grand Final Day date (#699)

* CHANGES sync

* Add queen's funeral holidays for canada (#710)

* CHANGES sync

* New Zealand updates for Queen Elizabeth II mourning + King Charles III (#708, #709)

* Australia - new king updates

* Fix typo in comment for Cuba (#712)

* CHANGES sync - v.0.16 release date, dedication

Co-authored-by: Akos <afurton@apple.com>
Co-authored-by: Akos Furton <furton.akos@bcg.com>
Co-authored-by: Maurizio Montel <mmontel@mmontel.mxp.csb>
Co-authored-by: David Hotham <david.hotham@blueyonder.co.uk>
Co-authored-by: Kate <kate@kgthreads.com>
Co-authored-by: Ark <ark@cho.red>
Co-authored-by: Bailey Thompson <bailey.kyle.thompson@gmail.com>
Co-authored-by: g-gg <52978382+g-gg@users.noreply.github.com>
Co-authored-by: Jacob Punter <54587331+JPunter@users.noreply.github.com>
Co-authored-by: Nikita <theedandroid@gmail.com>
Co-authored-by: Ryan McCrory <ryan.mccrory92@gmail.com>
dr-prodigy added a commit that referenced this pull request Nov 13, 2022
* add honduras tests

* add honduras tests

* Add tests for many edge cases

* Add tests for many edge cases

* remove duplicate function

* v.0.15 beta init

* Typechecking (#661)

* typechecking HolidayBase, holidays.utils, miscellaneous typechecking
note: coverage to be extended to other parts of the project code

* Update Ukraine holidays.

  - Add Day of Ukrainian Statehood holiday.
  - Edit Victory Day name.
  - Fix formatting.
  - Update tests for Ukraine holidays.
  - Switch to <= and >= for consistency.
  - Rename Orthodox Easter.

* CHANGES sync

* Add Day after Thanksgiving to US - PA holidays (#675)

* CHANGES sync

* CHANGES sync, pre-commit reviews

* test_ukraine pre-commit fix

* Bump actions/setup-python from 3.1.0 to 4.1.0 (#682)

* Bump pre-commit/action from 2.0.3 to 3.0.0 (#672)

* removed py36 from toml target-version array

* Add Cuba (#678)

authored-by: bthompson <bthompson@pinterest.com>

* CHANGES sync

* Make Colombian holidays preserve historical observation (#676)

Authored-by: bthompson <bthompson@pinterest.com>

* Make Venezuela holidays preserve historical observation (#677)

Authored-by: bthompson <bthompson@pinterest.com>

* CHANGES sync

* black fixes

* removed outdated deprecation warnings

* Drop support for PortugalExt

* Ensure Canada Christmas observed falls after 25 and not before - new version #579

* CHANGES sync

* README sync (support for Cuba)

* Drop support for UK subdivisions as countries (England, Scotland..) (dr-p)
Drop support for IsleOfMan as UK subdivision (dr-p)

* Added support for Python 3.11, improved README badges

* Fixed CD/CI setup.cfg typo

* Added py3.11 to toml config

* Removed py3.11 from setup.cfg due to failing pre-commit checks

* Bump pypa/gh-action-pypi-publish from 1.5.0 to 1.5.1 #686

* Doc example fix (#685)

* added pentecost monday for province of bolzano

source: https://it.wikipedia.org/wiki/Pentecoste

* Whitsun Monday - province of Bolzano - IT (#689)

* Bump actions/setup-python to 4.2.0 (#688)

* v.0.15

* v.0.16 beta init

* Add UK Holiday for Queen Elizabeth II Funeral (#702)

* Add Moldova (#695)

* Add Bolivian holidays (#698)

* CHANGES sync

* Fix NYSE New Year observance for Saturdays. (#696)

* CHANGES sync

* Financial market support review, new method financial_holidays(..) #694

* Add Australia National Day of Mourning and confirmed Grand Final Day date (#699)

* CHANGES sync

* Add queen's funeral holidays for canada (#710)

* CHANGES sync

* New Zealand updates for Queen Elizabeth II mourning + King Charles III (#708, #709)

* Australia - new king updates

* Fix typo in comment for Cuba (#712)

* CHANGES sync - v.0.16 release date, dedication

* Fix Honduras

* Add 2014 test honduras

* Rename Eswatini from Swaziland

* Add import

* Add pre-commit isort hook.

* Run `pre-commit run --all-files`.

* Fix black - isort conflict.

* Revert docs conf.py changes.

* Sort

* Sort

* Introduce special holidays support.

Extend `HolidayBase` in order to make special holidays handling simpler.

* Fix documentation and UK special holidays list.

* Add a test.

* Add Bosnia and Herzegovina holidays.

* Edit Eid ul-Adha holiday logic and improve code readability.

* Improve readability.

* Add more tests.

* Update README.rst

* Change `special_holidays` format.

* Add Liechtenstien holidays and tests.

* Refactor tests.

* Remove empty constructors.

Move country holidays sources/descriptions from comments to docstrings.

* v.0.17 beta init

* Sort holiday names. (#713)

* Sort holiday names.
Order holidays by name when there are multiple holidays on the same date.

* CHANGES sync

* CHANGES sync

* Supported countries no. update, mexico.py permissions fix

* Keep context based imports sorting for `holidays.constants`.

* Fix observed holiday for christmas and boxing day canada (#715)

* CHANGES sync

* .pre-commit-config.yaml update

* Get holidays range type fix

* CHANGES sync

* README.rst: Fix documentation about Portugal subdiv param (#742)

* Bump actions/setup-python from 4.2.0 to 4.3.0 (#740)

Bumps [actions/setup-python](https://github.com/actions/setup-python) from 4.2.0 to 4.3.0.
- [Release notes](https://github.com/actions/setup-python/releases)
- [Commits](actions/setup-python@v4.2.0...v4.3.0)

---
updated-dependencies:
- dependency-name: actions/setup-python
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Fixed incorrect subdivision code (#733)

* CHANGES sync

* Run isort.

* Bug fix of substitute holidays in Japan (#723)

* CHANGES sync

* isort commit

* isort commit

* CHANGES sync #650, #728

* Fix Honduras Morazan weekend bug. (#744)

* CHANGES sync

* README fix for Liechtenstein

* Singapore: Revised Date for Vesak Day in 2023 (#726)

* CHANGES sync

* Update malaysia.py (#736)

fix month for deewali 2022

* CHANGES sync

* README fix: pip install --upgrade (#735)

* Update Ukrainian holidays (#743)

* CHANGES sync

* Redesign special holidays population logic.

* Fix `_populate` docstring.

* README.rst: Remove Alpha-3 code for Bosnia and Herzegovina (#755)

Adding an extra entry like the Alpha-3 code in the countries
table causes render failure

* Added observed holidays for Ukraine (#746)

* Implement 100% test coverage (#747)

* Fix a typo: Liechtenstien -> Liechtenstein. (#752)

* Fixed Easter holidays for Bulgaria (#748)

* CHANGES sync

* pyproject.toml fixed, Ukraine reformatting

* CHANGES sync

* Portugal: add district holidays (#753)

Added holidays for the 18 portuguese districts
Added tests
Updated README.rst

* Import isort reformatting

* Fix Diwali date for 2022. (#754)

* CHANGES sync

* Optimizations for Ireland (#756)

* CHANGES sync

* Remove extra spaces from strings (#759)

* CHANGES sync

* Mexico: optimizations (#760)

* CHANGES sync

* Germany: optimizations (#766)

* China: optimizations (#767)

* CHANGES sync

* Fix a couple of `_populate` related issues.

* United Kingdom: optimizations (#780)

* Ukraine: observed holidays calculation more universal and readable (#776)

* Canada: optimizations, Yukon Heritage Day fix (#775)

* CHANGES sync

* Clean up Ireland tests. (#765)

Remove `print` statement.

* add documentation to _populate (see #704) (#763)

* Fix observance logic for laborable holidays in Uruguay (#758)

* Fix `list_supported_countries` and `list_supported_financial` methods. (#764)

Replace `obj.__base__` with `issubclass`.
Add Isle Of Man alpha-3 code.

* brazil.py: Update SC holiday according 2017 law (#761)

From 2017 onwards if the 11th of August falls on a work day the
holiday is moved to the next Sunday.

Issue: 592

* Added special case on 2022 (#779)

Added special case on 2022 Chinese Mid-Autumn Festival

Added test case on test_hongkong.py

* Remove `assert` from Norway and Sweden. (#771)

Rework all Sundays collection logic (use dateutil.rrule).
Extract repetitive test_sundays code to a separate file.

* South Africa: (#773)

- replace while loops with dateutil logic (#768)
- Queen's Birthday day fix
- observed holidays calculation fix
- added tests

* Replace Python built-in asserts with unittest asserts. (#777)

* CHANGES sync

* Replace while loops with dateutil logic (#768) (#770)

* CHANGES sync

* Lithuania: add the 2nd on November as holiday (#781)

* CHANGES sync

* MyPy implicit optional option

* Update Hong Kong holiday, mid-autumn (#782)

* CHANGES sync

* Sweden: fix Midsummer evening (#783)

* CHANGES sync

* v0.17

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Akos <afurton@apple.com>
Co-authored-by: Akos Furton <furton.akos@bcg.com>
Co-authored-by: Maurizio Montel <mmontel@mmontel.mxp.csb>
Co-authored-by: David Hotham <david.hotham@blueyonder.co.uk>
Co-authored-by: Kate <kate@kgthreads.com>
Co-authored-by: Ark <ark@cho.red>
Co-authored-by: Bailey Thompson <bailey.kyle.thompson@gmail.com>
Co-authored-by: g-gg <52978382+g-gg@users.noreply.github.com>
Co-authored-by: Jacob Punter <54587331+JPunter@users.noreply.github.com>
Co-authored-by: Nikita <theedandroid@gmail.com>
Co-authored-by: Ryan McCrory <ryan.mccrory92@gmail.com>
Co-authored-by: Pedro Baptista <32106544+Nalguedo@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Michael Thessel <der.thes@gmail.com>
Co-authored-by: Sho Hirose <sho.hirose@gmail.com>
Co-authored-by: Mike Borsetti <26698111+mborsetti@users.noreply.github.com>
Co-authored-by: shahonseven <shahonseven@gmail.com>
Co-authored-by: ~Jhellico <KJhellico@users.noreply.github.com>
Co-authored-by: poshingchu <59795066+poshingchu@users.noreply.github.com>
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.

4 participants