Refactor Burundi holidays #925
Conversation
KJhellico
force-pushed
the
upd-burundi
branch
2 times, most recently
from
13dd35c
to
303336b
Compare
KJhellico
force-pushed
the
upd-burundi
branch
from
303336b
to
e901559
Compare
KJhellico
force-pushed
the
upd-burundi
branch
from
e901559
to
3d9d253
Compare
| for dt in _islamic_to_gre(year, 12, 10): | ||
| self[dt] = "Eid al Adha" | ||
|
|
||
| if self.observed: |
There was a problem hiding this comment.
I know we have similar code in other 15+ countries and it may look like a good approach (the code is compact and quite readable). However, that's not the way the observed holidays handling should be implemented.
As python-holidays is a somewhat performance oriented library :) the goal is to focus on compatibility, performance, and code readability (probably, in this specific order).
The problem is in O(m*n) complexity for observed holidays logic. The more years you want to cover the longer it'll take (e.g., Burundi(years=range(1900, 2000) vs Burundi(years=range(0, 2000)). In general we try to avoid such kind of approaches.
I'm going to merge this PR as is as we'll need to tackle the entire problem anyway so one more country won't make a big difference. The main idea is to avoid applying the approach in future PRs.
There was a problem hiding this comment.
I thought about it. I want to try remake some of such countries with _add_with_observed() mechanism (and individual processing rare cases).
|
Merged into beta! |
No description provided.