facebook / prophet

Tool for producing high quality forecasts for time series data that has multiple seasonality with linear or non-linear growth.
https://facebook.github.io/prophet
MIT License
18.57k stars 4.54k forks source link

Drop Korea class from hdays.py #1437

Closed chongdae closed 4 years ago

chongdae commented 4 years ago

Curent lunar caleder for Korea class is based on the "Chinese" lunar calender, which is differ from the "Korean" lunar calender.

Recently the python-holidays package is updated and fixes the problem. https://github.com/dr-prodigy/python-holidays/pull/294

Please drop Korea class and use the class in python-holidays instead.

cc @seriousran @1kko

seriousran commented 4 years ago

@chongdae

Good point! I'm going to replace Korea class with python-holidays. Which released version of python-holidays includes Korea class?

ref: https://github.com/dr-prodigy/python-holidays/releases

seriousran commented 4 years ago

I found it! :)

seriousran commented 4 years ago

@chongdae @1kko

I got an error like below. Should I install and import the korean_lunar_calendar package? Would you mind if you add this dependency to python-holidays? It seems better.

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-10-838628385e95> in <module>()
      1 import holidays as hdays_part1
      2 
----> 3 holiday_names = getattr(hdays_part1, 'KR')(years=2019).values()

3 frames
/usr/local/lib/python3.6/dist-packages/holidays/countries/korea.py in get_solar_date(self, year, month, day)
    187     # convert lunar calendar date to solar
    188     def get_solar_date(self, year, month, day):
--> 189         self.korean_cal.setLunarDate(year, month, day, False)
    190         return date(self.korean_cal.solarYear, self.korean_cal.solarMonth,
    191                     self.korean_cal.solarDay)

AttributeError: 'KR' object has no attribute 'korean_cal'
chongdae commented 4 years ago

@seriousran

The holidays package already has dependency to korean_lunar_calendar.

https://github.com/dr-prodigy/python-holidays/blob/master/setup.py#L40

seriousran commented 4 years ago

Thanks! I got it!

bletham commented 4 years ago

This fix has been pushed.