Revert XKRX lunar/solar calendar based holidays to hardcoded dates

[maread99]

Given that we have a list of Korean holidays hardcoded in, how would people feel about removing all of the Korean Lunar/Solar calendar stuff? The code's not thread-safe at initialisation time, and it's relatively slow to do its computations. I'm happy to open up the PR removing it, and add any remaining necessary hardcoded holidays.

_Originally posted by @philiptromans in https://github.com/gerrymanoim/exchange_calendars/issues/125#issuecomment-1003392281_

[maread99]

Hi @philiptromans.

XKRX is certainly slow.

Would you know how many annual holidays, that are currently evaluated automatically from consideration of lunar/solar calendars, would require being hardcoded instead?

Quite a lot of work on the current implementation was undertaken by @elbakramer who might want to comment on the proposed changes?

Reverting these holidays to hardcoded dates would be @gerrymanoim's call.

If you were to go ahead with a PR, perhaps you'd be good enough to bear in mind bug #94 (perhaps it'd come out in the wash if these holidays are changed back to hardcoded dates?).

Cheers

[maread99]

@philiptromans, I saw your latest PR #152 (thanks!) and the suggestion again that XKRX is moved to hardcoded holidays.

I notice @elbakramer hasn't commented here. Also there was no response to a couple of open issues that I asked for input on back in October (#94 and #95). Given all of which, I thnk it's fair to say elbakramer's probably hasn't got an ongoing interest in the calendar.

Personally, I would also prefer to move to hardcoded dates. If the calendar is being used then it's not a lot of work for someone to update it each year. In return we'd:

• get a standard implementation that everyone's familiar with.
• very substantially speed up the constructor.
• lose the korean_lunar_calendar dependency.
• hopefully fix the late_opens bug (#94).

@gerrymanoim, what do you reckon, should we move to hardcoded holidays if @philiptromans is happy to PR it?

[gerrymanoim]

Hey sorry for the delay. Fine by me! If it is causing us maintenance headaches, let's move it back to hardcoded.

[philiptromans]

Ok, sounds good. I'll try to sort this at some point. It'll likely be the next time I notice a problem with the calendar though.

On Fri, 4 Mar 2022, 22:18 gerrymanoim, @.***> wrote:

Hey sorry for the delay. Fine by me! If it is causing us maintenance headaches, let's move it back to hardcoded.

— Reply to this email directly, view it on GitHub https://github.com/gerrymanoim/exchange_calendars/issues/128#issuecomment-1059571485, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOXVI76HWXCKKPERPVAZJ3U6KD3JANCNFSM5LFBVFXA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

[maread99]

In PR #177 I've proposed moving all the special offsets implementation from ExchangeCalendar to XKRXExchangeCalendar. It was originally incorporated to ExchangeCalendar for the benefit of XKRXExchangeCalendar. No other calendar uses it.

If XKRXExchangeCalendar were to be moved to hardcoded dates then I suspect all this could be lost from XKRXExchangeCalendar, together with the apply_special_offsets method that I've left on ExchangeCalendar as a hook into the constructor.

[maread99]

Worth noting that if this were to go to hardcoded dates then I believe the whole of the pandas_extensions directory would be made redundant. (EDIT 24-06-24 exchange_calendar_xmos.py now uses .pandas_extensions.offsets.MultipleWeekmaskCustomBusinessDay, i.e. would need to leave at least this functionality somewhere)

TestXKRXCalendar.test_feb_29_2022_in_lunar_calendar

(bit of an aside but kind of related. Noted here for reference.)

The above test is currently raising a DeprecationWarning:

exchange_calendars\exchange_calendars\pandas_extensions\korean_holiday.py:75: DeprecationWarning: The truth value of an empty array is ambiguous. Returning False, but in future this will result in an error. Use `array.size > 0` to check that an array is not empty.
    if dt >= since:

The dt parameter of korean_holiday.alternative_holiday_func(dt, is_already_holiday, since) is occasionally receiving a pd.DatetimeIndex as opposed to a pd.Timestamp. The reason's not obvious to me - the callers are somewhere within the modules of pandas_extensions. Also, I haven't been able to recreate the warning outside of the test. EDIT - see #300.

[maread99]

Also, see #311 including comment re broken update_xkrx_holidays script.