Add GetISOWeekOfYear()

[iSazonov]

Rationale

.Net don't implement ISO 8601 standard - it have alternative scheme in:

System.Globalization.Calendar.GetWeekOfYear(DateTime time, CalendarWeekRule rule, DayOfWeek firstDayOfWeek);

Unix date utility implements ISO 8601:

date +%V

In PowerShell repo we use workaround described here (see Keno's comment) for:

Get-Date -UFormat %V

.Net should have parity and native implementation.

ISO 8601 is culture-independent so we can use static methods.

If we are going in the direction I believe it make sense consider all ISO8601 objects and compatibility with Unix date utility.
https://en.wikipedia.org/wiki/ISO_week_date#Relation_with_the_Gregorian_calendar

• first week
• last week
• weeks per year

http://man7.org/linux/man-pages/man1/date.1.html

• %g last two digits of year of ISO week number (see %G)
• %G year of ISO week number (see %V); normally useful only with %V
• %V ISO week number, with Monday as first day of week (01..53)

Proposed API

namespace System.Globalization
{
    public abstract class Calendar : ICloneable
    {
        public static class ISOWeek
        {
            public static int GetWeekOfYear(DateTime time);
            public static int GetYear(DateTime time);
            public static DateTime GetYearStart(int year);
            public static DateTime GetYearEnd(int year);
            public static int GetWeeksInYear(int year);
            public static DateTime ToDateTime(int year, int week, DayOfWeek day);
    }
}

Please note that the proposed API is a static and not instance member. The reason is ISO week is culture and calendar independent.

[svick]

Why not just add a new member for ISO to CalendarWeekRule?

[khellang]

@svick And then ignore firstDayOfWeek (unless it's Monday)? ISO 8601 specifies Monday as first day of week.

It's basically the existing GetWeekOfYear with CalendarWeekRule.FirstFourDayWeek + DayOfWeek.Monday and a tiny bit of behavior change. See https://blogs.msdn.microsoft.com/shawnste/2006/01/24/iso-8601-week-of-year-format-in-microsoft-net/

[svick]

@khellang

Probably a better choice would be to require that firstDayOfWeek is Monday (and throw otherwise).

But at that point, the original proposal of adding a new method might be better.

[tarekgh]

I believe it is better to add a new member to CalendarWeekRule enum. Something like Iso8601 or Iso8601ModayFirstDayOfWeek. I don't think we need to add a new API for that. if you agree, could you please update the proposal?

CC @ShawnSteele

[tarekgh]

The other idea could be adding the new member Iso8601 to the CalendarWeekRule enum

And then, have an overload to the method

System.Globalization.Calendar.GetWeekOfYear(DateTime time, CalendarWeekRule rule);

which assume Monday is the first day of the week.

And Make System.Globalization.Calendar.GetWeekOfYear(DateTime time, CalendarWeekRule rule, DayOfWeek firstDayOfWeek); to throw if firstDayOfWeek is not Monday and rule is Iso8601.

we may list all alternative proposals and we can discuss that with the design comittee.

[iSazonov]

Assume that the third parameter should be Monday and otherwise throw an exception - this case does not look convenient. I don't see the benefit of this exception.
We could ignore the third parameter silently if the second parameter is Iso8601.
Or we could make the third parameter optional with Monday default.

[tarekgh]

@iSazonov this why I suggested the other idea to have the overload

System.Globalization.Calendar.GetWeekOfYear(DateTime time, CalendarWeekRule rule);

which assume Monday as the first day of the week.

I would list all alternative design options in the description of this issue and then we can choose the best of them when discussing it with the design committee.

[iSazonov]

I'll update the proposal with your ideas.

[iSazonov]

@tarekgh Proposed API updated. Feel free to correct if I skipped something.

[tarekgh]

Thanks, @iSazonov I have modified it a little but didn't change anything in the proposal.

[ShawnSteele]

Adding an Iso8601Week to the enumeration is a reasonable idea.
One peculiar note I've observed is that some applications want to know the ISO week, but then they ask locales/cultures what their week counting preference is. Which kind of works for some locales that have mapped to the ISO week rules, but then breaks for other locales.

[tarekgh]

@ShawnSteele what exactly your suggestion here regarding your peculiar note? I think Windows may need to start flagging the locales which need ISO weeks and the framework would map these locales to use the new enum value we introduce.

[ShawnSteele]

@tarekgh Two things:

1. Apps that explicitly want ISO week rules because that's how the app works probably need GetISOWeekOfYear(). It seems silly to me for an app that "needs" the ISO behavior to get a calendar for a culture/language and then try to correct it. They want explicit behavior, it should be reasonably easy.
2. Some cultures intend to have ISO week behavior. The framework should probably select the ISO week rule behavior when it sees Windows LOCALE_IFIRSTWEEKOFYEAR of "2" (first four day week).

[tarekgh]

Thanks, @ShawnSteele

Apps that explicitly want ISO week rules because that's how the app works probably need GetISOWeekOfYear()

So you are suggesting the new API shouldn't be part of the calendar class. it can be a static API on DateTimeFormInfo for instance. right?

Some cultures intend to have ISO week behavior. The framework should probably select the ISO week rule behavior when it sees Windows LOCALE_IFIRSTWEEKOFYEAR of "2" (first four day week).

This is implementation details, we can talk more about that when we are going to implement it. thanks for the feedback.

[ShawnSteele]

So you are suggesting the new API shouldn't be part of the calendar class. it can be a static API on DateTimeFormatInfo for instance. right?

That would probably be more appropriate, yes.

[tarekgh]

if this is the case, then we should remove the alternative design option (which suggest adding more values to the enum). and keep just one option which is introducing one static API. If all agree, I'll go and change the proposal according to that.

[ShawnSteele]

@tarekgh - I think that depends on what the main use case(s) are. In my experience many apps explicitly desire the ISO week, and the culture dependent behavior is irrelevant.

It is less clear to me if there is a reasonable use case where the culture's preference for an ISO week counting mechanism is interesting to an app even though other cultures may use different systems. The majority of apps seem to want the culture-independent ISO week behavior.

[khellang]

Is there such a thing as culture-dependent ISO week behavior? Doesn't that defeat the entire point of the standard? 🤔

Personally, I like where we're heading now much better than adding an enum value and throwing for some combination of arguments.

[tarekgh]

good, I'll update the proposal to have the new static API (I thinking to have it in the Calendar class for discoverability) and I'll remove the enum proposals. I believe that will be much clearer.

[ShawnSteele]

@khellang That's sort of my point. Some regions use the ISO week as their cultural practice as well. Sometimes there is then confusion about whether the software wants the ISO standard or common practice for the region.

It's silly to ask for the culture version of the ISO week, but it's fair to ask if the culture uses the ISO week by default.

[iSazonov]

If we are going in the direction I believe it make sense consider all ISO801 objects and compatibility with Unix date utility.
https://en.wikipedia.org/wiki/ISO_week_date#Relation_with_the_Gregorian_calendar

• first week
• last week
• weeks per year
• weeks per month

http://man7.org/linux/man-pages/man1/date.1.html

• -I output date/time in ISO 8601 format
• %g last two digits of year of ISO week number (see %G)
• %G year of ISO week number (see %V); normally useful only with %V
• %V ISO week number, with Monday as first day of week (01..53)

[tarekgh]

@iSazonov update the proposal if you want to add the other functionality too.

[iSazonov]

@tarekgh Done.

[tarekgh]

for

        // Not defined in ISO 8601 so may be remove
        public static int GetISOWeeksPerMonth(DateTime time);

I prefer to remove this as it is not defined in the ISO 8601. we have the option later to add it if it is really needed.

for

        //-I output date/time in ISO 8601 format
        public static string GetISODatetimeFormat(DateTime time);
       // Additionally or alternatively we could implement formatting method
        public static string FormatISODatetime(DateTime time);

We don't add formatting APIs here. also, did you look at https://docs.microsoft.com/en-us/dotnet/standard/base-types/standard-date-and-time-format-strings we are already supporting the ISO date formatting. look at "o" and "r" specifiers. I believe we should remove these APIs from the proposal. if you agree, please update the proposal one more time and we can proceed from here. thanks.

[iSazonov]

I agree.

[tarekgh]

What exactly

public static int GetISOYearOfISOWeek(DateTime time);

does? could you give some example?

[ShawnSteele]

Sorry, I'm mostly lurking on this thread, however I wanted to point out that ISO weeks are contiguous across the new year transition from Dec 31 to Jan 1. So week 1 of the new year may begin in December of the previous year, or week 53 may of one year may include Jan 1 of the following year.

In those cases the "week" belongs to either the previous or the next year, which could cause odd behavior like Jan 1 being part of the previous year's week. Which means that it is interesting to know which year that ISO week would be considered part of, eg: 1 Jan, 2005 is 2004-W53-6 for example.

Wikipedia has a reasonable summery of the behavior https://en.wikipedia.org/wiki/ISO_week_date

Note that if you aren't counting ISO weeks, then the year is just the Gregorian year, so an ISO formatted date for 1 Jan 2005 would still be 2005-01-01 - it's only when the week numbers are being used that the years get weird.

So "GetISOYear" isn't sufficient, since it only applies when counting by ISO week. I think that GetISOYearOfISOWeek is pretty descriptive and I can't think of anything better.

[tarekgh]

Thanks @ShawnSteele I marked the issue ready for review.

[tarekgh]

Thinking more about the proposed APIs, I think it would be better to introduce the ISOCalendar class instead of adding these static methods to the Calendar class. I mean we'll implement the full ISO calendar and add all needed functionality. what do you think about that?

[khellang]

What's the feeling on nested types? Calendar.ISO.GetWeekOfYear?

[terrajobst]

Looks good. ISO shouldn't be all caps but we already have globalization APIs with all caps, so that ship has sailed.

Nesting that type under calendar forces everyone to use really long qualified names (e.g. Calendar.ISOWeek.GetWeekOfYear() vs just ISOWeek.GetWeekOfYear()), hence promoting ISOWeek to a top-level type seems better, especially because it helps with discovery.

namespace System.Globalization
{
    public static class ISOWeek
    {
        public static int GetWeekOfYear(DateTime time);
        public static int GetYear(DateTime time);
        public static DateTime GetYearStart(int year);
        public static DateTime GetYearEnd(int year);
        public static int GetWeeksInYear(int year);
        public static DateTime ToDateTime(int year, int week, DayOfWeek day);
    }
}

[karelz]

@khellang I sent you collaborator invite so that we can assign the issue to you (GH limitation). Ping me once you accept (assigning to myself temporarily).
Pro-tip: Switch repo to "Not Watching" to avoid 500+ notifications daily (you will get automatically subscribed to all notifications as collaborator).

[khellang]

Done! I've been watching the repo for years anyway 😉

[tarekgh]

@terrajobst should this issue be marked as api-approved?

[karelz]

Yeah, this was approved (marking as such). @terrajobst's computer was just too slow to make the change :(

[khellang]

Just a quick question: should this type be added to System.Private.CoreLib (in coreclr) and wait for the code to sync with corefx and corert? And then add type forwarding and tests in System.Globalization?

[tarekgh]

Just a quick question: should this type be added to System.Private.CoreLib (in coreclr) and wait for the code to sync with corefx and corert? And then add type forwarding and tests in System.Globalization?

Yes, please add the code to the coreclr repo and please CC me on the PR. Thanks.

[iSazonov]

Many thanks!

[iSazonov]

@tarekgh Is the enhancement in 2.1.1?

[khellang]

It's assigned to the 3.0 milestone,so don't think so 🤔

[karelz]

Correct. 2.1.1 is servicing. We don't add features or non-critical fixes into servicing branches, that would destabilize them and make them basically another master branch.

[iSazonov]

Thanks! I see 3.0 milestone in PRs.

[karelz]

I usually do sweeps of incorrect milestones (like Future) 1x-2x each week for both issues and PRs.
I am a bit behind due to some high-pris on my plate, sorry. Will catch up as soon as I can.

[paulomorgado]

Isn't ISOWeek a violation of the framework naming guidelines?

[khellang]

@paulomorgado Read https://github.com/dotnet/corefx/issues/28933#issuecomment-392873426 a couple of comments above.

[khellang]

We don't add features or non-critical fixes into servicing branches, that would destabilize them and make them basically another master branch.

@karelz But will this ship with 2.2?

[paulomorgado]

Sorry @khellang, I as on the phonw.

[karelz]

But will this ship with 2.2?

Sadly, no. 2.2 is tactical release froked off release/2.1 branch in CoreFX repo (2.1 servicing) - note: That is repo specific decision, not necessarily reflected in other repos (ASP.NET, CLI, etc.). Master branch flows into .NET Core 3.0 in CoreFX repo.

[dasMulli]

Late to the party: was there (can't find) a discussion considering making a dedicated type (struct) for a week number? - IsoWeek / IsoWeekNumber / WeekNumber containing both Year and Week properties.
IMHO it seems more concise than needing to call two methods to get two numbers that represent a single thing.
Not sure how other people use week numbers, but I used to need both the week and year together in most cases.

[khellang]

was there a discussion considering making a dedicated type for a week number?

Nope. The proposal was always about methods for getting the information you need in different situations. Creating a type that wraps these numbers would be a different proposal.

IMHO it seems more concise than needing to call two methods to get two numbers that represent a single thing.

Which calls are you referring to? What information do you have and what information are you interested in?

If you have "a single thing" that is comprised of two numbers, it makes a lot of sense to split those methods. Writing a wrapping struct and composing these methods at a later time is trivial.

[ffes]

Also a bit late to the party, but why is the return values one int? Know that 2016-01-02 fell in week 2015.53 and 2019-12-31 will be in week 2020.01

I still can not use something like this:

int week = ISOWeek.GetWeekOfYear(new DateTime(2016, 1, 2));

I do need to check something like every time I need to know a week:

var d = new DateTime(2016, 1, 2)
int week = ISOWeek.GetWeekOfYear(d);
int year = d.Year;
if (d.Month == 1 && week >= 52)
    year--;
if (d.Month == 12 && week == 1)
   year++;

Why not return a Tuple?

public static (int year, int week) GetWeekOfYear(DateTime date);

[khellang]

Why not return a Tuple?

@ffes If you have var d = new DateTime(2016, 1, 2), you should be able to do

var year = ISOWeek.GetYear(d);
var week = ISOWeek.GetWeekOfYear(d);

If you want to roll that into a single method returning a tuple, that should be pretty easy to do.

[ffes]

I missed the existence of GetYear(). Sorry for the noise 😌