Problems with easter about holidays

[ppeble]

Hello all.

This is by far the oldest bug that I inherited when I took over this gem. I feel like I can safely state that I have this licked. Unfortunately the delay was because the solution required a large refactoring. We no longer simply store regions in a @@cache variable and we don't simply require regions when they are looked up. The gem still stores regions and caches in-memory (like @@cache) but in a much more formalized way that just shoving it into a class variable.

I have also reworked how methods like easter are calculated behind the scenes in a way that I feel is more programmatic. The old way that things like easter worked was to simply require the method into the main Holidays namespace. This is all well and good when we have few such methods but over time that method count grew and that, combined with all of the region-specific methods, made it hard to keep the state of the program in your head. We now store methods as procs in-memory and also cache their results for specific regions and dates. This makes it much easier to understand the state of the gem as it is being used.

I am confident that the situations with the order of loading regions as stated above is no longer an issue. I will be closing this issue after the impending release of 4.0.0, which I think will be tomorrow or early this coming. Since I've made changes to the public API (4.0.0 is a breaking change and will probably require code changes on the part of the callers) I would prefer that a new bug be opened so we can investigate it together.

Thanks again for your patience. I'll wait until the official 4.0.0 is up on Rubygems before closing this.

In closing I will say that if my refactoring took away a method that was useful to you (like Holidays.easter) then please open an issue and let me know. I'm happy to discuss adding back to the public API. I decided to be very aggressive in pruning what I exposed to the public.

[ppeble]

4.0.0 is out! I'm gonna close this, please open a new bug after testing the new version and we can troubleshoot there.

Thanks for your patience!

[sesam]

I can verify the problem:
irb
require 'holidays'
Holidays.easter(2013).holidays
=> []
Holidays.easter(2013).holidays :gb
=> [{:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"Easter Sunday", :regions=>[:gb]}]
1.9.3-p392 :003 > Holidays.easter(2013).holidays
=> [{:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"Easter Sunday", :regions=>[:gb]}]

It seems that i need to seek holidays by a specific region first, before trying any non-region-specific holiday search.

[ppeble]

It's shameful that this was sitting for so long but I'm looking into this tonight. Let me run through each point brought up and say what I found.

date=Date.civil 2012, 4, 8
=> Sun, 08 Apr 2012

Holidays.on date,:us,:observed
=> []

This seems to be desired behavior. According to our definitions for the :us region, Easter Sunday is considered an 'informal' holiday. If you were to run Holidays.on date,:be,:observed or even Holidays.on date,:be then the holiday would appear. According to the US public holidays wiki page the federal government doesn't recognize Easter as a true holiday since it always falls on a Sunday and they are mainly concerned with marking 'official' holidays that occur on working days.

development environment (Rails 3.2.3)
irb(main):001:0> d=Date.civil 2012,7,4
=> Wed, 04 Jul 2012
irb(main):002:0> d.holidays
=> []
irb(main):003:0> d.holidays :us
=> [{:date=>Wed, 04 Jul 2012, :name=>"Independence Day", :regions=>[:us]}]
d.holidays
=> [{:date=>Wed, 04 Jul 2012, :name=>"Independence Day", :regions=>[:us]}]

I am unable to replicate this on plain irb but I can replicate on Rails 3.2.12! Uh oh.

I have the same problem with Rails with these:

require 'holidays'
Holidays.easter(2013).holidays
=> []
Holidays.easter(2013).holidays :gb
=> [{:date=>#, :name=>"Easter Sunday", :regions=>[:gb]}] 
1.9.3-p392 :003 > Holidays.easter(2013).holidays
=> [{:date=>#, :name=>"Easter Sunday", :regions=>[:gb]}]

This makes me nervous! I'll try to understand what is going on here.

EDIT: I take that back, I can replicate this in plain irb in some cases. What I am finding is that our unit tests aren't really independent. It's because we are relying on class variables for regions and holidays_by_month. I'm going to have to think about this one. 😦

[sesam]

Any known workarounds? For instance, could cache_between be used in a specified way to always get correct results?

[jufemaiz]

Ok, I can confirm that this occurs in POR as well and I'm certain I know the root cause.

require 'holidays'

puts Holidays.easter(2013).holidays.inspect
puts Holidays.easter(2013).holidays(:us).inspect
puts Holidays.easter(2013).holidays.inspect

gives the following:

[]
[]
[{:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"Easter Sunday", :regions=>[:ca]}, {:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"Cesar Chavez Day", :regions=>[:us_ca]}]

When you do some digging you'll find the problem is due to no regions having been loaded to be looked at.

require 'holidays'

puts Holidays.regions.inspect
puts Holidays.easter(2013).holidays.inspect
puts Holidays.regions.inspect
puts Holidays.easter(2013).holidays(:us).inspect
puts Holidays.regions.inspect
puts Holidays.easter(2013).holidays.inspect
puts Holidays.regions.inspect

gives us

[]
[]
[]
[]
[:ca, :ca_qc, :ca_ab, :ca_on, :ca_sk, :ca_mb, :ca_ns, :ca_pe, :ca_bc, :ca_nf, :ca_nt, :ca_nu, :ca_nb, :ca_yk, :mx, :mx_pue, :us, :us_ca, :us_dc]
[{:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"Easter Sunday", :regions=>[:ca]}, {:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"Cesar Chavez Day", :regions=>[:us_ca]}]
[:ca, :ca_qc, :ca_ab, :ca_on, :ca_sk, :ca_mb, :ca_ns, :ca_pe, :ca_bc, :ca_nf, :ca_nt, :ca_nu, :ca_nb, :ca_yk, :mx, :mx_pue, :us, :us_ca, :us_dc]

The cause is the use of a cache of regions via @@Regions array in the Holidays class, which is then appended with any additional regions being looked at.

I believe that the best approach is to ensure that you have preloaded the regions you're wanting before calling the holidays function without a region specified.

require 'holidays'
Holidays.load_all
puts Holidays.regions.inspect
puts Holidays.easter(2013).holidays.inspect
puts Holidays.regions.inspect
puts Holidays.easter(2013).holidays(:us).inspect
puts Holidays.regions.inspect
puts Holidays.easter(2013).holidays.inspect
puts Holidays.regions.inspect

gives

[:ar, :at, :au, :au_nsw, :au_vic, :au_qld, :au_nt, :au_act, :au_sa, :au_qld_brisbane, :au_qld_cairns, :au_tas_south, :au_wa, :au_tas, :au_tas_north, :be, :br, :ca, :ca_qc, :ca_ab, :ca_on, :ca_sk, :ca_mb, :ca_ns, :ca_pe, :ca_bc, :ca_nf, :ca_nt, :ca_nu, :ca_nb, :ca_yk, :us, :ch_zh, :ch_be, :ch_lu, :ch_ur, :ch_sz, :ch_ow, :ch_nw, :ch_gl, :ch_zg, :ch_fr, :ch_so, :ch_bs, :ch_bl, :ch_sh, :ch_ar, :ch_ai, :ch_sg, :ch_gr, :ch_ag, :ch_tg, :ch_ti, :ch_vd, :ch_ne, :ch_ge, :ch_ju, :ch_vs, :ch, :cl, :cr, :cz, :de, :de_bw, :de_by, :de_he, :de_nw, :de_rp, :de_sl, :de_st, :de_bb, :de_mv, :de_sn, :de_th, :dk, :ecb_target, :el, :es_pv, :es_ct, :es_na, :es_v, :es_vc, :es_an, :es, :es_ib, :es_cm, :es_mu, :es_m, :es_ar, :es_cl, :es_cn, :es_lo, :es_ga, :es_ce, :es_o, :es_ex, :fr, :gb, :gb_eng, :gb_wls, :gb_eaw, :gb_nir, :gb_sct, :gb_con, :je, :gb_jsy, :gg, :gb_gsy, :im, :gb_iom, :hr, :hu, :ie, :is, :it, :li, :lt, :nl, :no, :pl, :pt, :ro, :sk, :si, :fedex, :federal_reserve, :fi, :jp, :ma, :mx, :mx_pue, :nerc, :us_ca, :us_dc, :nyse, :nz, :nz_sl, :nz_we, :nz_ak, :nz_nl, :nz_ot, :nz_sc, :nz_hb, :nz_mb, :nz_ca, :nz_ch, :nz_wl, :ph, :se, :sg, :united_nations, :ups, :ve, :vi, :za]
[{:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"Easter Sunday", :regions=>[:au_nsw, :au_vic, :ca, :gb, :ph]}, {:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"Pâques", :regions=>[:be]}, {:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"Páscoa", :regions=>[:br, :pt]}, {:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"Påskedag", :regions=>[:dk]}, {:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"Uskrs", :regions=>[:hr]}, {:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"Páskadagur", :regions=>[:is]}, {:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"Pasqua", :regions=>[:it]}, {:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"Šv. Velykos", :regions=>[:lt]}, {:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"Pasen", :regions=>[:nl]}, {:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"1. påskedag", :regions=>[:no]}, {:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"Niedziela Wielkanocna", :regions=>[:pl]}, {:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"velikonočna nedelja", :regions=>[:si]}, {:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"Pääsiäispäivä", :regions=>[:fi]}, {:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"Påskdagen", :regions=>[:se]}, {:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"Cesar Chavez Day", :regions=>[:us_ca]}]
[:ar, :at, :au, :au_nsw, :au_vic, :au_qld, :au_nt, :au_act, :au_sa, :au_qld_brisbane, :au_qld_cairns, :au_tas_south, :au_wa, :au_tas, :au_tas_north, :be, :br, :ca, :ca_qc, :ca_ab, :ca_on, :ca_sk, :ca_mb, :ca_ns, :ca_pe, :ca_bc, :ca_nf, :ca_nt, :ca_nu, :ca_nb, :ca_yk, :us, :ch_zh, :ch_be, :ch_lu, :ch_ur, :ch_sz, :ch_ow, :ch_nw, :ch_gl, :ch_zg, :ch_fr, :ch_so, :ch_bs, :ch_bl, :ch_sh, :ch_ar, :ch_ai, :ch_sg, :ch_gr, :ch_ag, :ch_tg, :ch_ti, :ch_vd, :ch_ne, :ch_ge, :ch_ju, :ch_vs, :ch, :cl, :cr, :cz, :de, :de_bw, :de_by, :de_he, :de_nw, :de_rp, :de_sl, :de_st, :de_bb, :de_mv, :de_sn, :de_th, :dk, :ecb_target, :el, :es_pv, :es_ct, :es_na, :es_v, :es_vc, :es_an, :es, :es_ib, :es_cm, :es_mu, :es_m, :es_ar, :es_cl, :es_cn, :es_lo, :es_ga, :es_ce, :es_o, :es_ex, :fr, :gb, :gb_eng, :gb_wls, :gb_eaw, :gb_nir, :gb_sct, :gb_con, :je, :gb_jsy, :gg, :gb_gsy, :im, :gb_iom, :hr, :hu, :ie, :is, :it, :li, :lt, :nl, :no, :pl, :pt, :ro, :sk, :si, :fedex, :federal_reserve, :fi, :jp, :ma, :mx, :mx_pue, :nerc, :us_ca, :us_dc, :nyse, :nz, :nz_sl, :nz_we, :nz_ak, :nz_nl, :nz_ot, :nz_sc, :nz_hb, :nz_mb, :nz_ca, :nz_ch, :nz_wl, :ph, :se, :sg, :united_nations, :ups, :ve, :vi, :za]
[]
[:ar, :at, :au, :au_nsw, :au_vic, :au_qld, :au_nt, :au_act, :au_sa, :au_qld_brisbane, :au_qld_cairns, :au_tas_south, :au_wa, :au_tas, :au_tas_north, :be, :br, :ca, :ca_qc, :ca_ab, :ca_on, :ca_sk, :ca_mb, :ca_ns, :ca_pe, :ca_bc, :ca_nf, :ca_nt, :ca_nu, :ca_nb, :ca_yk, :us, :ch_zh, :ch_be, :ch_lu, :ch_ur, :ch_sz, :ch_ow, :ch_nw, :ch_gl, :ch_zg, :ch_fr, :ch_so, :ch_bs, :ch_bl, :ch_sh, :ch_ar, :ch_ai, :ch_sg, :ch_gr, :ch_ag, :ch_tg, :ch_ti, :ch_vd, :ch_ne, :ch_ge, :ch_ju, :ch_vs, :ch, :cl, :cr, :cz, :de, :de_bw, :de_by, :de_he, :de_nw, :de_rp, :de_sl, :de_st, :de_bb, :de_mv, :de_sn, :de_th, :dk, :ecb_target, :el, :es_pv, :es_ct, :es_na, :es_v, :es_vc, :es_an, :es, :es_ib, :es_cm, :es_mu, :es_m, :es_ar, :es_cl, :es_cn, :es_lo, :es_ga, :es_ce, :es_o, :es_ex, :fr, :gb, :gb_eng, :gb_wls, :gb_eaw, :gb_nir, :gb_sct, :gb_con, :je, :gb_jsy, :gg, :gb_gsy, :im, :gb_iom, :hr, :hu, :ie, :is, :it, :li, :lt, :nl, :no, :pl, :pt, :ro, :sk, :si, :fedex, :federal_reserve, :fi, :jp, :ma, :mx, :mx_pue, :nerc, :us_ca, :us_dc, :nyse, :nz, :nz_sl, :nz_we, :nz_ak, :nz_nl, :nz_ot, :nz_sc, :nz_hb, :nz_mb, :nz_ca, :nz_ch, :nz_wl, :ph, :se, :sg, :united_nations, :ups, :ve, :vi, :za]
[{:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"Easter Sunday", :regions=>[:au_nsw, :au_vic, :ca, :gb, :ph]}, {:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"Pâques", :regions=>[:be]}, {:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"Páscoa", :regions=>[:br, :pt]}, {:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"Påskedag", :regions=>[:dk]}, {:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"Uskrs", :regions=>[:hr]}, {:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"Páskadagur", :regions=>[:is]}, {:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"Pasqua", :regions=>[:it]}, {:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"Šv. Velykos", :regions=>[:lt]}, {:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"Pasen", :regions=>[:nl]}, {:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"1. påskedag", :regions=>[:no]}, {:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"Niedziela Wielkanocna", :regions=>[:pl]}, {:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"velikonočna nedelja", :regions=>[:si]}, {:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"Pääsiäispäivä", :regions=>[:fi]}, {:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"Påskdagen", :regions=>[:se]}, {:date=>#<Date: 2013-03-31 ((2456383j,0s,0n),+0s,2299161j)>, :name=>"Cesar Chavez Day", :regions=>[:us_ca]}]
[:ar, :at, :au, :au_nsw, :au_vic, :au_qld, :au_nt, :au_act, :au_sa, :au_qld_brisbane, :au_qld_cairns, :au_tas_south, :au_wa, :au_tas, :au_tas_north, :be, :br, :ca, :ca_qc, :ca_ab, :ca_on, :ca_sk, :ca_mb, :ca_ns, :ca_pe, :ca_bc, :ca_nf, :ca_nt, :ca_nu, :ca_nb, :ca_yk, :us, :ch_zh, :ch_be, :ch_lu, :ch_ur, :ch_sz, :ch_ow, :ch_nw, :ch_gl, :ch_zg, :ch_fr, :ch_so, :ch_bs, :ch_bl, :ch_sh, :ch_ar, :ch_ai, :ch_sg, :ch_gr, :ch_ag, :ch_tg, :ch_ti, :ch_vd, :ch_ne, :ch_ge, :ch_ju, :ch_vs, :ch, :cl, :cr, :cz, :de, :de_bw, :de_by, :de_he, :de_nw, :de_rp, :de_sl, :de_st, :de_bb, :de_mv, :de_sn, :de_th, :dk, :ecb_target, :el, :es_pv, :es_ct, :es_na, :es_v, :es_vc, :es_an, :es, :es_ib, :es_cm, :es_mu, :es_m, :es_ar, :es_cl, :es_cn, :es_lo, :es_ga, :es_ce, :es_o, :es_ex, :fr, :gb, :gb_eng, :gb_wls, :gb_eaw, :gb_nir, :gb_sct, :gb_con, :je, :gb_jsy, :gg, :gb_gsy, :im, :gb_iom, :hr, :hu, :ie, :is, :it, :li, :lt, :nl, :no, :pl, :pt, :ro, :sk, :si, :fedex, :federal_reserve, :fi, :jp, :ma, :mx, :mx_pue, :nerc, :us_ca, :us_dc, :nyse, :nz, :nz_sl, :nz_we, :nz_ak, :nz_nl, :nz_ot, :nz_sc, :nz_hb, :nz_mb, :nz_ca, :nz_ch, :nz_wl, :ph, :se, :sg, :united_nations, :ups, :ve, :vi, :za]

[jufemaiz]

Hey @ptrimble thanks for the hard work!!!