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

add kr holidays #63

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

add kr holidays #63

wants to merge 9 commits into from

Conversation

aanoaa
Copy link

@aanoaa aanoaa commented Mar 17, 2021

Repulic of Korea holidays contains lunar calendar.

  • LunarNewYear
  • BuddhasDay
  • Chuseok

and the Korean lunar calendar is different from that of China.

Copy link
Owner

@rickar rickar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this is cool. I've never looked at lunar calendar stuff before.

@@ -50,6 +50,7 @@ type Holiday struct {
Julian bool // the holiday is based on a Julian calendar
Observed []AltDay // the substitution days for the holiday
Func HolidayFn // logic used to determine occurrences
Lunar bool // the holiday is based on a Lunar calendar
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see this used anywhere. Is it needed?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -0,0 +1,9 @@
root = true
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please delete this file. We use whatever "go fmt" dictates.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure i'll patch.

@@ -0,0 +1,1232 @@
package kr
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this generated somehow? What was the source? Is there some way to calculate this at runtime rather than have this huge file?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll find a way.

some of data from..
https://www.kasi.re.kr/kor/index
(Korea Astronomical Research Institute)

but i can't found this meta data today.


and chinese lunar calendar data depends on https://github.com/godcong/chronos/blob/master/data.go
I haven't verified it.

}
)

// CalcDayOfLunarMonth calculates the occurrence of a holiday that is always a
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think these would be used by other lunar calendars? Maybe these functions along with data.go and lunar.go should be moved to something like 'aa/lunar' where it can be shared?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

korean, chinese and japanese lunar calendars are different.
each lunar calendar should implements on they requirements.

I don't think it needs to be shared.

return time.Date(y, time.Month(m), d, 0, 0, 0, 0, cal.DefaultLoc)
}

func julianDayToDate(jd int) (year, month, day int) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are already Julian functions in cal_funcs.go. Do those not work?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh i missed it.

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

Successfully merging this pull request may close these issues.

2 participants