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

proposal: time: add Epiweek #42002

Closed
jmeekhof opened this issue Oct 15, 2020 · 8 comments
Closed

proposal: time: add Epiweek #42002

jmeekhof opened this issue Oct 15, 2020 · 8 comments
Labels
Milestone

Comments

@jmeekhof
Copy link

@jmeekhof jmeekhof commented Oct 15, 2020

I'd like to propose adding epiweek to the time package.
An epiweek is almost identical to an ISO Week. The only difference is that an epiweek begins on Sunday and an ISO Week begins on Monday.

Adding this functionality should be trivial and would be beneficial for certain branches of work.

@ianlancetaylor ianlancetaylor changed the title Add Epiweek to the time package proposal: time: add Epiweek Oct 15, 2020
@gopherbot gopherbot added this to the Proposal milestone Oct 15, 2020
@gopherbot gopherbot added the Proposal label Oct 15, 2020
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Oct 15, 2020

This seems easy to write in a third party package. See https://golang.org/doc/faq#x_in_std.

@jmeekhof
Copy link
Author

@jmeekhof jmeekhof commented Oct 15, 2020

Implementation wise, it's identical to ISOWeek, and that is in the standard package. The only difference is you pivot on Wednesday instead of Thursday. Access to the private abs() methods make the implementation trivial in the package.

func (t Time) EpiWeek() (year, week int) {
	// According to the rule that the first calendar week of a calendar year is
	// the week including the first Thursday of that year, and that the last one is
	// the week immediately preceding the first calendar week of the next calendar year.

	// weeks start with Sunday
	// Sunday Monday Tuesday Wednesday Thursday Friday Saturday
	// 1      2       3         4        5      6        7
	// +3     +2      +1        0        -1     -2       -3
	// the offset to Wednesday
	abs := t.abs()
	d := Wednesday - absWeekday(abs)
	// handle Saturday
	if d == 4 {
		d = -3
	}
	// find the Wednesday of the calendar week
	abs += uint64(d) * secondsPerDay
	year, _, _, yday := absDate(abs, false)
	return year, yday/7 + 1
}
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Oct 15, 2020

Seems like an external package could use ISOWeek and adjust if January 1 is a Wednesday.

My point is that since this can be written in an external package, we need to justify adding it to the standard library. What is that justification? Ease of implementation is not a justification; new API requires building and testing and maintenance and requires people reading the package to read and understand the functionality. It's not free. Of course we should do it if it's important, but why is Epiweek important?

@jmeekhof
Copy link
Author

@jmeekhof jmeekhof commented Oct 15, 2020

Points are well put. Epiweeks is an epidemiological week. It is often used for counting weeks and for forecasting flu outbreaks across the country.

The question I would ask is, why was it considered important to add ISOWeek to the time package? If they're the same thing with a different starting day of the week, why wouldn't they be in the same package?

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Oct 15, 2020

Because ISO weeks are widely used. I don't know how widely used Epiweeks are. If they are only used by epidemiologists, then I'm not sure that they are widely used.

For example, there is a Wikipedia entry for ISO weeks (https://en.wikipedia.org/wiki/ISO_week_date) but as far as I can tell there is not one for Epiweeks.

@jmeekhof
Copy link
Author

@jmeekhof jmeekhof commented Oct 15, 2020

I wish the US epidemiologist used ISO weeks, it would make all of our lives easier. Epiweeks are mostly used in data science work, and you see support for this variation of week counting in languages such as R and Python.

https://rdrr.io/cran/lubridate/man/week.html

https://epiweeks.readthedocs.io/

https://www.cdc.gov/epiinfo/user-guide/check-code/epiweekfunctions.html

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Oct 15, 2020

The Python example is a third party package. The same could be done for Go.

@jmeekhof
Copy link
Author

@jmeekhof jmeekhof commented Oct 16, 2020

I implemented this as a third party package.
jmeekhof/epiweek

@jmeekhof jmeekhof closed this Oct 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.