GothenburgBitFactory / holidata

Holidata is the core of holidata.net, a no-nonsense, ad-free provider of international holiday data.
https://holidata.net
MIT License
47 stars 13 forks source link

Add locale sv-SE #48

Closed Zegnat closed 4 years ago

Zegnat commented 4 years ago

Closes #19.

This adds all the holidays based on 1989:253. As @pxtimes3 pointed out in #19 – where a short list of dates was posted – there are a number of “afton” (english: eve) days on some lists that do not have a basis in this law.

However I did chose to include julafton, midsommarafton, and nyårasfton, because these are specifically defined by law to be Sundays. Specifically by 1977:480 § 3 a ¶ 2. Because Holidata lists dates “on which business or work are suspended or reduced”, and many businesses treat these days as weekends, I felt it made sense to include them.

This is a different law. Vacation law rather than national holidays. And I was not sure if Holidata wanted to separate the two. For separation sake I have not marked these three days as [N]. May that be a valid solution? How have other countries handled this?

Zegnat commented 4 years ago

I found some minor issues which need to be addressed before merging. See also the comments in the code.

Should all be resolved now! Please do let me know if there is anything else. I do not usually write Python code, so all tips and tricks are very much welcome.

There is no separation between holidays: If it is nation-wide, it deserves an N 😉 .

I noticed that nl-NL dropped the N on some days. These days (dodenherdenking, Sinterklaas) also happened to be days that aren’t holidays defined by the state (e.g. both are missing on the official list for 2020). So I kinda stole the idea from there.

I have no real opinion on the matter though. So if merging is dependent on adding back the Ns, let me know!

Zegnat commented 4 years ago

Sorry to bother you again, but the N flag for Nyårsafton, Midsommarafton and Julafton is still missing... 😬

Yep, as I said in my previous comment, this was inspired by a setup in the nl-NL file. It would be really helpful if someone could define exactly what N means and when to add it. Or more importantly, when not to add it, as it currently seems like it is just a superfluous default flag.

On this occasion, perhaps you could squash your PR into a single commit

Done. (Rebased on master, all commits squashed.) I was keeping changes separate for easier review on yours end. If you do not need it, I will keep it all to a single commit.

Hope this addresses everything.

recollir commented 4 years ago

One of the original project creators here. We initially started to use N to indicate holidays and de-facto holidays (people being off work) on a national level. Without the N the holiday or de-facto holiday would be on a regional level. Just take a look at Switzerland. https://holidata.net/de-CH/2020.csv That said, yes a better documentation about the intent would be good to have.

BTW I am also benefiting from this PR as well - being in Gbg. Have been procrastinating the holidays for SE for too long. Thanks for doing it.

Zegnat commented 4 years ago

I might just do a PR for nl-NL then too. As far as I know there is nothing regional about dodenherdenking. (On the other hand, I also do not think it is a day people generally get off work anywhere…)

I am in Gothenburg too. And is was actually tomorrow’s holiday that made me file this PR. Because I didn’t know I was going to get off work for it 😉

recollir commented 4 years ago

Corrections are always welcomed. It is almost impossible to get these right without local knowledge.

lauft commented 4 years ago

It would be really helpful if someone could define exactly what N means and when to add it. As @recollir already said: N stands for national and should be given to holidays which are not restricted to (ISO_3166-2) subregions.

The definition of the holiday flags stems from the beginning of the holidata project when the files were more or less curated by hand. With the change to the algorithmic method, it is likely some errors slipped through. There is definitely some cleanup to do.

lauft commented 4 years ago

@Zegnat I took the opportunity to add additional tests for holiday flags to the code (865550726a847c468ca31d595af47e8ddd1b1529) . This also revealed some more mismatches apart of those you mentioned (#51, #52, #53, #54, #55). These are now fixed as far as the current definitions are valid.

Once again thanks for your contribution. Maybe you can keep an eye on the Swedish legislation and inform us on any future holiday changes. 😉

Zegnat commented 4 years ago

Those tests look good! Will definitely make it a lot easier for first time contributors like me to help out. Thanks a tonne, @lauft!

Once again thanks for your contribution. Maybe you can keep an eye on the Swedish legislation and inform us on any future holiday changes. 😉

I am looking at integrating the holidata output with my plain text calendar. So that should help me find any future mismatches when my employer tells me I have the day off without it being on my calendar 😉 That situation led me here in the first place because of midsommar this week.