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

Feature request: support multiple country holidays #909

Open
eromoe opened this issue Mar 30, 2019 · 6 comments
Open

Feature request: support multiple country holidays #909

eromoe opened this issue Mar 30, 2019 · 6 comments

Comments

@eromoe
Copy link

eromoe commented Mar 30, 2019

Hello,

This is because in some county like China Japan , people also would enjoy some foreign holidays .
For example , chocolate sales would grow on Valentine's Day .
Cake and apple(yes apple) sales would have large increase on Christmas Eve .

Current behaviour

>>> m.add_country_holidays('CN')
>>> m.add_country_holidays('US')
WARNING:fbprophet:Changing country holidays from CN to US

Add a flag to determine whether appending or changing would be much better .

@bletham
Copy link
Contributor

bletham commented Apr 2, 2019

We had a lot of discussion about this when we were adding the country holidays feature. The big complication is with holidays that exist / overlap for both countries. Here are some examples of where things get weird:

  • US and Canada both have a holiday named "Thanksgiving". This is not the same holiday and usually occurs on different days. So in this case we would need to change all US holidays to be "{holiday} US" and CA holidays to "{holiday} CA" so that we don't end up merging the Thanksgivings into one holiday.

  • Both countries also have the holiday "New Year's Day", which is the same holiday. Splitting this into "New Year's Day US" and "New Year's Day CA" would be not-ideal because then we have an identifiability issue where we have two parameters that always show up together. So here we want to actually merge across countries.

  • ES has the holiday "Año nuevo". This is the same holiday as "New Year's Day". As above, this should really be merged with "New Year's Day" and not treated as a separate holiday since the dates overlap perfectly.

Now maybe reasonable things will happen even if these identical holidays are not merged, but I think that'd need some extensive testing. Handling these in the "right" way (don't merge Thanksgivings, do merge New Years and Año nuevo) will be very hard to do without making a table of equivalent holidays.

I'll leave this open as an enhancement to look into more in the future, but that's why there isn't the option to automatically stack country holidays.

In the meantime, you can do this manually and make your own decision for which holiday should be merged or not. Here is how you get a holiday dataframe with country holidays:

from fbprophet.make_holidays import make_holidays_df

year_list = [2016, 2017, 2018, 2019, 2020]
holidays = make_holidays_df(year_list=year_list, country='CN')

You could then do the same thing for US, and then stack them after handling equivalent holidays as you'd like. Pass that dataframe in to Prophet(holidays=holidays).

@goncaloperes
Copy link

goncaloperes commented Jul 4, 2020

@bletham this SO thread may be useful as it considers one of the examples on how to handle the holidays.
In this case, the question (and answer) will help people that only want to know in which dates for multiple regions there will be an holiday, and it merges the holidays that have the same date in the row of the first one that appears for that specific date.

This is how the output looks like:
image

@saccpp
Copy link

saccpp commented Aug 17, 2020

Hello, Is France holidays available in the make_holidays_df?

@bletham
Copy link
Contributor

bletham commented Aug 17, 2020

Most holidays comes from the holidays package, and France is included there: https://github.com/dr-prodigy/python-holidays

@brendan-r
Copy link

brendan-r commented Dec 3, 2020

Bit off topic, but @saccpp, "FR" fails due to the holidays package, but the data's there.

Until this PR lands to correct it, you can use "FRA" instead of "FR".

@saccpp
Copy link

saccpp commented Dec 4, 2020 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants