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

fixes #28 where making time struct with New() function did not counted leap year logic #31

Merged
merged 4 commits into from
Aug 17, 2024

Conversation

abtinokhovat
Copy link
Contributor

New(time) function was not calculating correctly on days which were leap days or in leap years

`tl := time.Date(2025, 3, 20, 0, 0, 0, 0, time.Local)
ti2 := New(tl)
fmt.Println(ti2.Format("yyyy-MM-dd"))

ti := Date(1403, Esfand, 28, 0, 0, 0, 0, time.Local)
ti = ti.AddDate(0, 0, 2)

fmt.Println(ti.Format("yyyy-MM-dd"))

`

for example these days are the same how ever the calculation had errors both have to be 1403-12-30 but the one with ptime package is 1404-01-01

also this fixes this issue.

@abtinokhovat abtinokhovat changed the title fix issue 27: bug where making time struct with New() function did not counted leap year logic fix: issue 27 bug where making time struct with New() function did not counted leap year logic Aug 3, 2024
@abtinokhovat abtinokhovat changed the title fix: issue 27 bug where making time struct with New() function did not counted leap year logic fix: issue 28 bug where making time struct with New() function did not counted leap year logic Aug 3, 2024
@yaa110
Copy link
Owner

yaa110 commented Aug 4, 2024

@abtinokhovat Thank you for fixing the issue. Could you please add some unit tests for it as well?

@abtinokhovat
Copy link
Contributor Author

Yep, of course, I will add some unit tests in near future.

@seyedmmousavi
Copy link

seyedmmousavi commented Aug 17, 2024

@abtinokhovat & @yaa110,
It is a good idea to document the calculation reference. Based on which method and mathematical formula have you done the calculations?

@abtinokhovat
Copy link
Contributor Author

I'm now adding some documentation to the mathematic formula's I've used here and I will also add some tests here.

@abtinokhovat
Copy link
Contributor Author

@seyedmmousavi @yaa110 I've added documentation as best as I could. The formula is somewhat ambiguous, and the result is based on my work on calendars from various sources over the years. I've included them as I remembered and had them. It is legacy code that I originally wrote in TypeScript and then rewrote in Go. I also added detailed test cases for each conversion function, both forward and backward.

@yaa110 yaa110 changed the title fix: issue 28 bug where making time struct with New() function did not counted leap year logic fixes #28 where making time struct with New() function did not counted leap year logic Aug 17, 2024
@yaa110
Copy link
Owner

yaa110 commented Aug 17, 2024

fixes #28

@yaa110 yaa110 merged commit 076e874 into yaa110:main Aug 17, 2024
1 check passed
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.

3 participants