Runtime: Add GetISOWeekOfYear()

Created on 9 Apr 2018  路  139Comments  路  Source: dotnet/runtime

Rationale

.Net don't implement ISO 8601 standard - it have alternative scheme in:
```c#
System.Globalization.Calendar.GetWeekOfYear(DateTime time, CalendarWeekRule rule, DayOfWeek firstDayOfWeek);

Unix `date` utility implements ISO 8601:

date +%V

In [PowerShell repo ](https://github.com/PowerShell/PowerShell/pull/6542) we use workaround described [here](https://blogs.msdn.microsoft.com/shawnste/2006/01/24/iso-8601-week-of-year-format-in-microsoft-net/) (see Keno's comment) for:
```powershell
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

c# 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.

Hackathon api-approved area-System.Globalization up-for-grabs

Most helpful comment

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

All 139 comments

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

@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/

@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.

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

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.

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.

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

C# 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.

I'll update the proposal with your ideas.

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

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

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.

@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.

@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).

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.

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.

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.

@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.

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

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

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.

@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.

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)

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

@tarekgh Done.

for

```C#
// 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 

```C#
        //-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.

I agree.

What exactly

```C#
public static int GetISOYearOfISOWeek(DateTime time);

```
does? could you give some example?

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.

Thanks @shawnsteele I marked the issue ready for review.

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?

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

@tarekgh Taking into account that ISO calendar is culture-independent and proposed methods is static your suggestion looks very good and the API will be easily discoverable.

@khellang

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

Do you mean ISO would be a static class? This may be ok but I am still preferring to have IsoCalendar class instead. the reason, is I am seeing ISO is a calendar by itself (like the Gregorian calendar) and can be used as any other calendar. even the calendar resources (the standard, books, sites...etc.) always refer to it as a calendar.

@iSazonov

Taking into account that ISO calendar is culture-independent and proposed methods is static your suggestion looks very good and the API will be easily discoverable.

I agree it is culture independent but this is doesn't really matter as we already have some calendars that are not related to specific cultures (e.g. JulianCalendar). discoverability issue is not different than any other calendar we support today. I would argue, anyone interested in the ISO functionality would look for ISO calendar instead of trying to find some methods of calendar class.

FWIW, I like the idea of Calendar.ISO - Similar to the way there is Encoding.UTF8. Presumably it could be an ISOCalendar.

However, there still is GetISOYearForISOWeek() which makes it extra functionality than other calendars would.

It almost makes me think there needs to be another ISOWeekCalendar form, because I'm not sure what happens to an ISOCalendar when you pass it into formatting APIs. How would the system know to format it as 2005-01-01 (ISO date format) or 2004-W53-6 (ISO week format)

I like the idea of Calendar.ISO - Similar to the way there is Encoding.UTF8. Presumably, it could be an ISOCalendar.

With that, we'll need ISOCalendar anyway. Having extra property Calendar.ISO we can have if it is proved really useful. But we didn't do that with other calendars. Imean we don't have Calendar.Gregorian for instance. In general, I think it can be a good idea as users don't have to create a new Calendar object every time they want to use it. and instead, we can return the cached calendar instance.

However, there still is GetISOYearForISOWeek() which makes it extra functionality than other calendars would.

We already have same cases with the Lunisolar calendars which expose more methods specific to those calendars.

It almost makes me think there needs to be another ISOWeekCalendar form because I'm not sure what happens to an ISOCalendar when you pass it into formatting APIs. How would the system know to format it as 2005-01-01 (ISO date format) or 2004-W53-6 (ISO week format)

ISO calendar would be culture independent and will not be used in the formatting or parsing at all. similar to Julian calendar.

Yes, I mean having a nested static class insidd Calendar with methods specific to ISO. As we've already discussed earlier in the thread, it doesn't make much sense to implement some of the existing methods (or at least overloads) of the existing Calendar abstract class.

@khellang

Is there any other calendar API beside Calendar.GetWeekOfYear(DateTime time, CalendarWeekRule rule, DayOfWeek firstDayOfWeek); you think will be problematic for the ISO calendar? I am asking because I am thinking to let the users use the ISO as any other regular calendar would be an advantage instead of having users to special case interacting with the ISO calendar. We can define the behavior of Calendar.GetWeekOfYear(DateTime time, CalendarWeekRule rule, DayOfWeek firstDayOfWeek); and we'll have another API Calendar.GetWeekOfYear(DateTime time);

@tarekgh Hmm, that's an interesting point. "CalendarWeekRule" is meaningless in this context. What would be the right behavior? Ignore it? Do something else?

TwoDigitYearMax is meaningless in an ISO calendar as ISO requires 4 digit years.

GetDaysInYear() for an ISO Week Calendar would have 52 or 53 weeks of days, right? That might be pecuiliar. And would be different than a non-ISO-Week calendar. And I'm not sure what that means for ISO Week calendar "months", as those are kind of meaningless concepts in an ISO week calendar. (The weeks are the months, kind-of)

GetLeapMonth begs the question of whether the 52/53 week year in an ISO week calendar has the concept of leap weeks?

ISO provides the concept of date formats in a traditional 2018-04-16 date format, or in the week of year format, 2018-W16-1 - the Calendar object can be used in a DTFI.Calendar. Which means that it could be used for formatting.

Normally DTFI specifies how a calendar is formatted and the Calendar provides the math. However, the key thing about the ISO calendar is that ISO explicitly defines the formatting (though the resolution of the format can vary). The variations permitted are around the resolution and the single variable of the ISO format vs ISO Week format.

Any solution probably needs to not confuse the ISO/formatting question.

TwoDigitYearMax is meaningless in an ISO calendar as ISO requires 4 digit years.

I believe this can behave as Gregorian calendar anyway. if ISO is using 4-digit year, then TwoDigitYearMax will not affect anything in the app calculation. I am not seeing a real problem here.

GetDaysInYear() for an ISO Week Calendar would have 52 or 53 weeks of days, right? That might be peculiar. And would be different than a non-ISO-Week calendar. And I'm not sure what that means for ISO Week calendar "months", as those are kind of meaningless concepts in an ISO week calendar. (The weeks are the months, kind-of)

Look at https://en.wikipedia.org/wiki/ISO_week_date it states"

An ISO week-numbering year (also called ISO year informally) has 52 or 53 full weeks. That is 364 or 371 days instead of the usual 365 or 366 days. The extra week is sometimes referred to as a leap week, although ISO 8601 does not use this term.

Which means the ISO year is either 364 or 371 days.

GetLeapMonth begs the question of whether the 52/53 week year in an ISO week calendar has the concept of leap weeks?

GetLeapMonth will always return 0 as there is no concept of a leap month in the ISO calendar.
IsLeapDay and IsLeapMonth will always return false. IsLeapYear can return true for the years has 371 days though.

ISO provides the concept of date formats in a traditional 2018-04-16 date format, or in the week of year format, 2018-W16-1 - the Calendar object can be used in a DTFI.Calendar. Which means that it could be used for formatting.
Normally DTFI specifies how a calendar is formatted and the Calendar provides the math. However, the key thing about the ISO calendar is that ISO explicitly defines the formatting (though the resolution of the format can vary). The variations permitted are around the resolution and the single variable of the ISO format vs ISO Week format.
Any solution probably needs to not confuse the ISO/formatting question.

The ISO calendar will not be associated with any culture (similar to Julian and Lunisolar calendars). So, formatting with this calendar wouldn't be any concern anyway.

For Calendar.GetWeekOfYear(DateTime time, CalendarWeekRule rule, DayOfWeek firstDayOfWeek); we can force the behavior to work by ignoring the CalendarWeekRule and DayOfWeek parameters. In addition, we'll introduce the new overload Calendar.GetWeekOfYear(DateTime time) for logical use. of course, we'll document the behavior of both methods and we can explain why these behaves like that.

I am not really pushing for having ISOCalendar, I am just seeing people are always referring to the ISO calculation as ISO calendar in the web and in the books (e.g. Calendrical Calculations). so this is why I thought it would make sense to have it as a calendar by itself, instead of having some static methods as a helper functions.

If you still see this is confusing, I am ok to explore more @khellang suggestion or stick with the last proposal.

Which means the ISO year is either 364 or 371 days.

Right, what I meant was that a .ISO calendar wouldn't be a .ISOWeek calendar. If it's only used for calculations, then we'd only need a .ISOWeek calendar (not the .ISO calendar) - but if the ISO calendar was added later, that'd be weird.

GetLeapMonth will always return 0 as there is no concept of a leap month in the ISO calendar.

There is a leap month in an ISO calendar. There is not a leap month in an ISO Week calendar. (Unless you used the month accessors? Are you planning to disable GetMonth() and the like?)

I'm thinking that an "ISO calendar" and "ISO Week Calendar" make sense from a formatting perspective - but you can make a DTFI that knows how to do ISO calendar formatting with any Gregorian calendar.

I'm also thinking that adding these as an explicit calendar or two seems sensible on the surface, but leads to lots of strange questions and potentially unexpected behavior. Or additional requests.

So, I'm leaning back toward adding the two methods to the Gregorian calendar: GetISOWeek() and GetIsoYearOfIsoWeek() (or GetIsoWeekYear()?) -- that would solve the immediate problem without causing a bunch of other odd behavior.

So, I'm leaning back toward adding the two methods to the Gregorian calendar: GetISOWeek() and GetIsoYearOfIsoWeek() (or GetIsoWeekYear()?) -- that would solve the immediate problem without causing a bunch of other odd behavior.

What is the benefit having these methods on the Gregorian calendar and not on the abstract Calendar class? or as @khellang suggested to have a static ISO class in Calendar class (i.e. Calendar.ISO.GetISOWeek)?

I'd assumed that Calendar.ISO would be a "full" ISOCalendar. But if it's just a few methods, that'd work I guess,

ISO would be a static class

```C#
namespace System.Globalization
{
public abstract class Calendar : ICloneable
{
public static class ISO
{
public static int GetISOWeekOfYear(DateTime time);
public static int GetISOFirstWeekOfYear(DateTime time);
public static int GetISOLastWeekOfYear(DateTime time);
public static int GetISOWeeksPerYear(DateTime time);
public static int GetISOYearOfISOWeek(DateTime time);
}
}

or we can stick with the original proposal 

```C#
namespace System.Globalization
{
    public abstract class Calendar : ICloneable
    {
        // All ISO 801 objects   (https://en.wikipedia.org/wiki/ISO_week_date#Relation_with_the_Gregorian_calendar)
        public static int GetISOWeekOfYear(DateTime time);
        public static int GetISOFirstWeekOfYear(DateTime time);
        public static int GetISOLastWeekOfYear(DateTime time);
        public static int GetISOWeeksPerYear(DateTime time);

        // Compatibility with Unix date utility (http://man7.org/linux/man-pages/man1/date.1.html)

        // %G year of ISO week number (see %V); normally useful only with %V
        public static int GetISOYearOfISOWeek(DateTime time);
    }
}

I'll update the proposal to include Calendar.ISO class and I'll list the original proposal as an alternative design.

@iSazonov if you are ok with that, could you update the proposal? or I can do it if you don't have any objection to change the proposal.

And once you have a Calendar.ISO static class, you can drop all the ISO pre- and suffixes from the method names 馃槉

@tarekgh Feel free to update - the proposal LGTM.

done!

I fixed typo in comments ISO 801 -> ISO 8601

One more question here regarding:

```C#
public static int GetFirstWeekOfYear(DateTime time);
public static int GetLastWeekOfYear(DateTime time);

Will GetFirstWeekOfYear always return 1? or we are returning the relative Gregorian week number here? 
Will GetLastWeekOfYear return GetWeeksPerYear returned value? or we are returning the relative Gregorian week number here? 

wouldn't be better to have these return DateTime object which returns when the first week starts and when last week end? May we change the names too to be GetYearStart and Get YearEnd? 

```C#
            public static DateTime GetFirstWeekOfYear(DateTime time);
            public static DateTime GetLastWeekOfYear(DateTime time);

Sorry, I haven't been following along :)
These sound bad to me. The "calendar" has a week setting which is NOT the ISO week. So if those are intended to be the first/last ISO weeks, they should say specify the iso week calendar.

No clue how the "first iso week" would be interesting, that's always "1", right? And the last week would be like "GetLastISOWeekOfYear" - which might be a little bit interesting.

The start/end of the iso year seem interesting to me since they're rarely Jan 1. You could also get the week # from that date if you want the last iso week # of the year.

IsoWeeksPerYear changes (52/53), so "Per" seems odd. There are 16 oz per quart (for every quart). IsoWeeksInYear?

GetIsoWeeksInYear is the same as the last ISO week of the year, so I'd prefer that the first/last go away, they're redundant.

I do like the idea of a:
public static DateTime GetISOWeekYearStart(DateTime time);
public static DateTime GetISOWeekYearEnd(DateTime time);

That is exactly what I was wondering about what is the benefit of having GetFirstWeekOfYear and GetLastWeekOfYear. so it looks to me these really not needed.

why don't you like returning the DateTime pointing to the beginning and the end of any year? I am not feeling strong about it, but at the same time, I can see the benefit of having these dates. Imagine you have a calendar control that can mark the start and the end of the ISO years. note that we can change the name of such APIs to something like GetYearStart and GetYearEnd.

You confused me. I did say:

I do like the idea of a:
public static DateTime GetISOWeekYearStart(DateTime time);
public static DateTime GetISOWeekYearEnd(DateTime time);

Obviously they'd have to specify the "ISOWeek" bit since they're only the ISO calendar.

sorry, I misread it as "I don't like".

What about the naming:

C# public abstract class Calendar : ICloneable { public static class ISO { ... public static DateTime GetYearStart(DateTime time); public static DateTime GetYearEnd(DateTime time); } }

That naming is insufficient.
An "ISO" Calendar is exactly like the Gregorian calendar we all know. It starts on 2018-01-01 and ends on 2018-12-31. It has 365/366 years.
This discussion is all about the ISO Week Calendar, which is different.

So, either the class should be called "ISOWeek" or the methods need to specify that.

@ShawnSteele looks you have missed these methods are inside ISO class already.

Would be Calendar.ISO.GetYearStart/GetYearEnd. or other suggestion is

Calendar.ISO.GetFirstWeekStart/GetLastWeekEnd

No, I didn't miss that. "ISO" specifies many different things about date formats. "ISO" does not imply that it MUST be the ISO Week Year format.

Either the class should be called ISOWeek, or the "Week" variation needs specified in the method.

eg: Calendar.ISOWeek.GetYearStart/GetYearEnd

Calendar.ISO.GetYearStart is ambiguous. I'd expect that to always be Jan 1, not the ISO Week Year Start

why Calendar.ISO.GetFirstWeekStart/GetLastWeekEnd is not ok?

That is assuming that the "first week" you're talking about is an ISO week. Which seems like a reasonable assumption for people that understand the ISO week calendar system, however I think it might be a little bit ambiguous to others.

I don't like ISOWeek name because the other API names in there wouldn't make sense. for example, having ISOWeek.GetWeekOfYear would have redundant "Week" name. If you think Calendar.ISO.GetFirstWeekStart/GetLastWeekEnd can be ambiguous, we can have Calendar.ISO.GetYearStart/GetYearEnd

The other methods use "ISOWeek"? So Calendar.ISO.GetFirstISOWeekStart?

GetYearStart/End I find less awkward, however they would need to include ISOWeek. GetISOWeekYearStart/End - which would make them more awkward.

The other methods use "ISOWeek"? So Calendar.ISO.GetFirstISOWeekStart?

No. we don't. look at the proposal in the top of the page and we have:

```C#
namespace System.Globalization
{
public abstract class Calendar : ICloneable
{
public static class ISO
{
// All ISO 8601 objects (https://en.wikipedia.org/wiki/ISO_week_date#Relation_with_the_Gregorian_calendar)
public static int GetWeekOfYear(DateTime time);
public static int GetFirstWeekOfYear(DateTime time);
public static int GetLastWeekOfYear(DateTime time);
public static int GetWeeksPerYear(DateTime time);

       // Compatibility with Unix date utility (http://man7.org/linux/man-pages/man1/date.1.html)

       // %G year of ISO week number (see %V); normally useful only with %V
        public static int GetYearOfWeek(DateTime time);
}

}
```

What about ISO.GetWeekYearStart/End?

I find "GetIsoFirstWeekStart" to be clumsy. You're really wanting the beginning/end of the ISO Week year. Which happens to be the beginning of the first week, but that's extra unnecessary mental redirection.

GetYearStart by itself is ambiguous because it doesn't say whether it's the calendar year or the week year.

What about ISO.GetWeekYearStart/End?

This looks reasonable to me. I'll update the proposal per that if nobody else object :-)

For GetWeekYearEnd, would this return the starting of last week in the year? or will return last day of the week of the year?

I'd return the start/end of the year. (last day of the week of the year for GetWeekYearEnd)

I asked because it asks for the Week of the year's end :-) I think this can be understood returning the first day of the week which end the year. The confusion I am seeing now is, does the API clear if we are returning the week or the day?

I started to think back GetYearStart/End would be clearer :-)

regarding your comment

GetYearStart by itself is ambiguous because it doesn't say whether it's the calendar year or the week year.

This will be clear because the call will have ISO word (e.g. ISO.GetYearStart) which indicate the year start of the ISO year. I am not seeing this is really ambiguous.

I would expect ISO.GetYearStart() to return 1 Jan and ISO.GetYearEnd() to return 31 Dec. This is the difference between an ISO year and an ISO week year.

Talked offline with @ShawnSteele and here is the final proposal we agreed on which should remove any confusion.

C# namespace System.Globalization { public abstract class Calendar : ICloneable { public static class ISOWeek { public static int GetWeekOfYear(DateTime time); public static DateTime GetYearStart(DateTime time); public static DateTime GetYearEnd(DateTime time); public static int GetWeeksPerYear(DateTime time); public static int GetYearOfWeek(DateTime time); } }

Let me know if anyone has any concern with the proposal so I can update the proposal at the top of the page.

LGTM.

Yeah, that looks good. Would it make sense to also include methods for formatting an ISO week date as a string?

From Wikipedia:

A precise date is specified by the ISO week-numbering year in the format YYYY, a week number in the format ww prefixed by the letter 'W', and the weekday number, a digit d from 1 through 7, beginning with Monday and ending with Sunday. For example, the Gregorian date 31 December 2006 corresponds to the Sunday of the 52nd week of 2006, and is written 2006-W52-7 (in extended form) or 2006W527 (in compact form).

Is there a reason GetWeeksPerYear, GetYearStart and GetYearEnd can't just take int year?

The existing methods on Calendar uses a different naming scheme; GetDaysInYear, GetDaysInMonth and GetMonthsInYear. Should GetWeeksPerYear instead be GetWeeksInYear for consistency?

Yeah, that looks good. Would it make sense to also include methods for formatting an ISO week date as a string?

Generally, formatting is done through the culture classes (e.g. DateTimeFormatInfo) but so far we don't support any culture need this formatting. we may think later providing similar to what we are providing for ISO formatting (i.e. "o" format) which can support YYYY-Wnn-dd (or YYYYWnndd). I like to do that separately from this proposal.

Is there a reason GetWeeksPerYear, GetYearStart, and GetYearEnd can't just take int year?

I thought about that before, I am thinking users would use ISO Week in conjunction with the Gregorian calendar. so I believe in main scenarios people would start with the DateTime and that is why I prefer to have these APIs takes DateTime and not the year number. let me know if you think otherwise.
Also, you have made me think of another scenario which I think we need to support, which is getting the Gregorian DateTime from ISO date. something like:

C# public DateTime ToDateTime(int year, int week, int day);

I think this is important to give flexibility converting from ISO to Gregorian. so people can roundtrip any dates. If you agree, I'll add this one to the proposal too.

The existing methods on Calendar uses a different naming scheme; GetDaysInYear, GetDaysInMonth and GetMonthsInYear. Should GetWeeksPerYear instead be GetWeeksInYear for consistency?

That is a good point. I'll update the API name to be GetWeeksInYear. thanks for the catch.

Shouldn't that be DayOfWeek day instead of int day? It's effectively "Calculating a date given the year, week number and weekday" outlined on Wikipedia

I believe in main scenarios people would start with the DateTime and that is why I prefer to have these APIs takes DateTime and not the year number. let me know if you think otherwise.

I tried implementing the methods in the current proposal and the aforementioned methods only need a Gregorian year. It's really a hassle when you have to construct an entire DateTime (with month and day) just to get the year start or end date.

Shouldn't that be DayOfWeek day instead of int day?

I'll update the proposal with that. you are right, the day should be between Sunday, Monday....Saturday

I tried implementing the methods in the current proposal and the aforementioned methods only need a Gregorian year. It's really a hassle when you have to construct an entire DateTime (with month and day) just to get the year start or end date.

The issue I am afraid people will run into is the input year, is it really the Gregorian year? or is it ISO year. you know we can get some dates in Gregorian (around Dec 29th) which can belong to the next year in ISO. having DateTime is removing this confusion and the Month and day number in DateTime is very important too to know the date map to which ISO year. The other proposal would be just accepting the year number which is the ISO year and not the Gregorian year and we can name the parameter as isoWeekYear.

my last reply was only regarding

C# public static DateTime GetYearStart(DateTime time); public static DateTime GetYearEnd(DateTime time);

but I still believe the reset has to take a DateTime and not just year.

the Month and day number in DateTime is very important too to know the date map to which ISO year.

It's important in some of the methods, yes, but not in the ones I mentioned. The only piece of information needed is a Gregorian year. It's trivial to pass DateTime.Year if you already have a DateTime instance, but it's totally unnecesary to construct a DateTime (which values do I use for month and day? does it matter? are they used?) just to pass it in.

BTW, there's already precedent for using int year in Calendar.GetDaysInYear. We might as well be consistent.

I can put up a gist with my quick implementations so you can get a feel for the APIs and their usage if you want?

It's important in some of the methods, yes, but not in the ones I mentioned. The only piece of information needed is a Gregorian year. It's trivial to pass DateTime.A year if you already have a DateTime instance, but it's totally unnecessary to construct a DateTime (which values do I use for month and day? does it matter? are they used?) just to pass it in.

What happens if you have a Gregorian year that crosses 2 ISO years? Here the example,

Sun 28 Dec 2008 | 2008-W52-7
Mon 29 Dec 2008 | 2009-W01-1

So if you pass 2008 to our APIs, it will be ambigous. This is why I mentioned the APIs has to take DateTime or it has to take the ISO year number and not the Gregorian number.

BTW, there's already precedent for using int year in Calendar.GetDaysInYear. We might as well be consistent.

I am not feeling strong about this one because this API will just return 7 * GetWeeksInYear(...)

One other note regarding using DayOfWeek for the day instead of int in the following API
C# public DateTime ToDateTime(int year, int week, int day);

ISO week usually using days from 1 to 7 while DayOfWeek having days from Sunday=0 to Saturday = 6. do you think this can create confusion? Also, if we use DayOfWeek, do you think we accept value like (DayOfWeek) 7?

One more thing, for the following 2 APIs

C# public static class ISOWeek { public static int GetWeekOfYear(DateTime time); public static int GetYearOfWeek(DateTime time); }

should we just call them GetWeek and GetYear? These will be written as ISOWeek.GetWeek(...) and ISOWeek.GetYear(...)

Perhaps it make sense to have:
c# public DateTime ToDateTime(int year, int week, int day); public DateTime ToDateTime(int year, int week, DayOfWeek day); public DateTime ToDateTime(int year, int weekOfYear); public DateTime ToDateTime(int year, int dayOfYear);

ISO week usually using days from 1 to 7 while DayOfWeek having days from Sunday=0 to Saturday = 6. do you think this can create confusion?

No, not at all. We already have an enum representing day of week. It would be a shame if we didn't use it. Why would we use a less specific type here, but a more specific type (DateTime) in GetWeeksInYear, GetYearStart and GetYearEnd?
I doubt anyone cares much whether Sunday represents 0, 1, 6 or 7. They just want Sunday, that's it. The API should be responsible for converting to the correct numeric representation.

Also, if we use DayOfWeek, do you think we accept value like (DayOfWeek) 7?

I'd say no. What would it represent? Are we assuming that they want Sunday?

should we just call them GetWeek and GetYear?

Yes, agreed 馃憤

What happens if you have a Gregorian year that crosses 2 ISO years? Here the example,

Sun 28 Dec 2008 | 2008-W52-7
Mon 29 Dec 2008 | 2009-W01-1

So if you pass 2008 to our APIs, it will be ambigous. This is why I mentioned the APIs has to take DateTime or it has to take the ISO year number and not the Gregorian number.

I still think you're confusing this with the other methods, which actually need the month and date.

This would be the result if you pass 2008 to the different methods:

  • GetWeeksInYear: 52
  • GetYearStart: 2007-12-31 | 2008-W01-1
  • GetYearEnd: 2008-12-28 | 2008-W52-7

And just for completeness, this is what 2009 would give you:

  • GetWeeksInYear: 53
  • GetYearStart: 2008-12-29 | 2009-W01-1
  • GetYearEnd: 2010-01-03 | 2009-W53-7

GetWeeksInYear will always return the number of weeks in the specified year. No matter what month and day it is. This calendar is still sitting on top of the Gregorian calendar.
GetYearStart and GetYearEnd will return the Gregorian start- and end date for the specified ISO week year.

Can you come up with an example where having a month and day actually would change these results?

Here's a gist with my current implementation:

https://gist.github.com/khellang/53b98851b1d23eef97503ebb22612b3c

@khellang

I'd say no. What would it represent? Are we assuming that they want Sunday?

Yes. The scenario in my mind is someone getting an ISO date from some source (which will have the days from 1..7) and then want just call the framework with. if we didn't treat 7 as Sunday, the caller has to do it (day % 7) before calling us.

I still think you're confusing this with the other methods, which actually need the month and date.

I think I am not confused or maybe I am still confused :-)
From your example it looks when you mentioned 2008, you meant 2008 is the ISO year and not the Gregorian year. That is what I tried to say the APIs takes the ISO year number. Look at my previous comment that I said or it has to take the ISO year number and not the Gregorian number..

For example:

Having someone have DateTime and just passing DateTime.Year to the APIs would be the wrong assumption. and they will need to do something like:

```C#
DateTime dt = new DateTime(2008, 12, 29);
DateTime yearStart = ISOWeek.GetYearStart(ISOWeek.GetYear(dt)); // ISOWeek.GetYear(dt) will return 2009 and not 2008.

This will return a different value than if you calling 

```C#
    DateTime yearStart = ISOWeek.GetYearStart(dt.Year);

Am I missing anything here? but if this is match what you are thinking, then I am fine to have the APIs takes just the year number and we'll need to clarify this year number is the ISO year number and not the Gregorian year number. Maybe we can name the parameter as isoYear or so.

I didn't look at your code but I'll take a look in the first chance.

@iSazonov

I think having

C# public DateTime ToDateTime(int year, int week, DayOfWeek day);

is enough here. I am not seeing the there suggested overloads is needed.

but if this is match what you are thinking, then I am fine to have the APIs takes just the year number and we'll need to clarify this year number is the ISO year number and not the Gregorian year number.

Is there even a difference between an "ISO week year" and a "Gregorian year"? I know they might start and end at different dates, but 2009 in the Gregorian calendar is 2009 in the ISO week calendar, right? It's only confusing once you start involving months and days.

At least ISOWeek.GetYearStart(ISOWeek.GetYear(dt)) can be composed nicely.

I'm not sure whether calling it year or isoYear would make it more or less confusing 馃槀

I'm not sure whether calling it year or isoYear would make it more or less confusing

If you think the confusion will still exist, then we may keep the original proposal of passing DateTime instead of the year. there would not be any confusion at that time.

The other idea can be having an overload for these APIs, one take DateTime and the other overload will take the year as you proposed. I still prefer calling the parameter as isoYear for that overload.

I just really struggle to understand how it makes it less confusing. It forces you to come up with some redundant input, while still passing in the same year integer. In my mind, that's not only as confusing, but also more work for no benefit.

Let's move on and confirm the APIs proposal that I think all of us agree on it now:

C# 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 isoYear); public static DateTime GetYearEnd(int isoYear); public static int GetWeeksInYear(int isoYear); public static DateTime ToDateTime(int isoYear, int week, DayOfWeek day); } }

does this sound right?

Should ToDateTime() be ToGregorianDateTime()?

How address "%G year of ISO week number (see %V); normally useful only with %V"? Should we add the overload:
c# public static int GetYearOfWeek(int isoYear, int isoWeek);

does this sound right?

Yes! Thank you, @tarekgh 鈾ワ笍

Sorry, I was out for a couple of days.

I'm not terribly fond of the "DayOfWeek". ISO Week Calendars have weeks equivalent to months of a traditional Gregorian calendar, and the day of week number is similar to the day of month number in a normal calendar. So it formats things like 2018-W12-7. We happen to know which "day" that 7 corresponds to, however the ISO Week format specifies day numbers. Sure, .Net has a DayOfWeek class, but this to me seems to lead to unnecessary munging of the values.

I'm also not terribly fond of the "isoYear" nomenclature. "Iso Year" isn't a thing. ISO specifies formatting of dates in an unambiguous traditional year-month-day form, or optionally in a year-week-day form. "Year" is a valid ISO concept for both systems. (And even additional systems). Indeed, ISO specifically states provides guidance for traditional Gregorian year numbers, such as requiring them to be 4 digit years to avoid Y2K confusion. By itself "Iso Year" could apply to either, though people sometimes shorten "Iso Week Calendar Year", which is a misleading convention..

The fact that this class is the IsoWeek class should be sufficient to determine that an ISO Week Calendar Year is meant by "year". If IsoWeek is too ambiguous still, then I'd suggest "IsoWeekCalendar" to further reduce confusion. If "iso" is felt to be needed, then why not "isoWeek" and "isoDay" as well? Particularly "week" has exactly the same qualifier that this is an Iso Week Calendar week and not some other week.

@ShawnSteele Thanks for chiming in again!

Sure, .Net has a DayOfWeek class, but this to me seems to lead to unnecessary munging of the values.

I don't think we should get too hung up on the values. The enum provides a way to represent "day of week", and it's used all over the .NET calendar/date/time APIs already. We should at least provide an overload with it, so it works seamlessly with the other APIs.

"isoYear" nomenclature. "Iso Year" isn't a thing. ISO specifies formatting of dates in an unambiguous traditional year-month-day form, or optionally in a year-week-day form. "Year" is a valid ISO concept for both systems. (And even additional systems). Indeed, ISO specifically states provides guidance for traditional Gregorian year numbers, such as requiring them to be 4 digit years to avoid Y2K confusion. By itself "Iso Year" could apply to either, though people sometimes shorten "Iso Week Calendar Year", which is a misleading convention..

The fact that this class is the IsoWeek class should be sufficient to determine that an ISO Week Calendar Year is meant by "year". If IsoWeek is too ambiguous still, then I'd suggest "IsoWeekCalendar" to further reduce confusion. If "iso" is felt to be needed, then why not "isoWeek" and "isoDay" as well? Particularly "week" has exactly the same qualifier that this is an Iso Week Calendar week and not some other week.

Yes, that's why I said:

Is there even a difference between an "ISO week year" and a "Gregorian year"? I know they might start and end at different dates, but 2009 in the Gregorian calendar is 2009 in the ISO week calendar, right? It's only confusing once you start involving months and days.

```c#
public static int GetWeekOfYear(DateTime time);
public static int GetYear(DateTime time);

Would it make sense to add overloads that accept `DateTimeOffset` instead of `DateTime`, to make these methods easier to use for people who have to care about time zones? In other words:

```c#
public static int GetWeekOfYear(DateTime time);
public static int GetWeekOfYear(DateTimeOffset time);
public static int GetYear(DateTime time);
public static int GetYear(DateTimeOffset time);

Would it make sense to add overloads that accept DateTimeOffset instead of DateTime, to make these methods easier to use for people who have to care about time zones?

What would the implementations do with the additional information (the UTC offset)?

@khellang

What would the implementations do with the additional information (the UTC offset)?

I think the same thing the DateTime overloads would do with DateTimeKind or TimeOfDay: ignore it.

I think the same thing the DateTime overloads would do with DateTimeKind or TimeOfDay: ignore it.

Doesn't that boil down to the same problem with having to pass a DateTime if you're only interested in the Year property?

Ideally, if .NET had a pure Date type, these overloads would just take that.

It's easy enough to go from a DateTimeOffset to a DateTime if that's all you need, no?

@khellang

It's easy enough to go from a DateTimeOffset to a DateTime if that's all you need, no?

It is. I was just wondering if making working with DateTimeOffset slightly more convenient would be worth it. This way, people are not discouraged to use DateTimeOffset when they should be using it.

This way, people are not discouraged to use DateTimeOffset when they should be using it.

I'm all for encouraging use of DateTimeOffset (when you need it), but in terms of consistency and number of overloads, I'm not sure it's worth it. Anyway, I don't have a strong opinion here. I'm sure the API reviewers will have some good arguments for or against.

For DateTimeOffset issue, I am not seeing ISOWeek is doing anything with time zones or with time in general. it mainly handles the date and not time. so I don't think we need to use DateTimeOffset.

For DayOfWeek issue, I like using the explicit type as DayOfWeek and we can still allow passing the value 7 as Sunday too. by doing that, there shouldn't be any problem to anyone need to call us with values from 1..7 (with casting to DayOfWeek) or anyone wants to call us with DayOfWeek Sunday..Saturday. I don't like to have the overload that takes a day of week number instead of DayOfWeek.

For ISO year, I am still seeing there is the difference between the Gregorian year and ISO year. obviously, ISO year can start in different time other than the Gregorian year. ISO year can have the length (number of days or number of weeks) different than the Gregorian year. It is enough difference to distinguish if we are assuming the input parameter is the Gregorian year or ISO year. assuming ISO year is a Gregorian year is a very wrong assumption. I have mentioned the previous example which can have the callers fool themselves when passing this parameter to our APIs. my stand here, either we should be clear naming the parameter as I suggested (or better name if you have one) to indicate the passed number indicate the ISO year and not the Gregorian year number. Or we have to let the API takes DateTime which clearly indicate the API is taking the Gregorian date and we can handle the edge cases according to the month and day numbers according to what we get from the DateTime value. I understand @khellang point that people don't have to construct the DateTime to just call our APIs, and I agree with him. This is why I accepted letting the API take the ISO year number and in the future, if we find any need for having overload API takes DateTime, we can add it.

I am still seeing there is the difference between the Gregorian year and ISO year.

Please, it is a Gregorian year and an ISO Week Year. ISO Years are ambiguous, either "Gregorian" or "iso week". For example, ISO 8601 specifies ordinal dates as well, eg: 2018-117. That is an 'ISO' format, and clearly in that ISO format 2018 would be the ISO year. But it's not the same as the ISO Week year.

If I'm constructing a DateTime from an ISO Week Date, the only thing that makes sense is to use the ISO Week Year. In which case "iso" in the year name is redundant. For that to be sensible it would need to be:

public static DateTime ToDateTime(int isoWeekYear, int isoWeekWeek, DayOfWeek isoWeekDay);

Except that we already said it is an "IsoWeek" class. so adding that is silly.

I have difficulty seeing a case where ToDateTime needs a normal Gregorian year, further implying that "year" meaning ISO Week Format Year is sufficient. If I'm passing the other 2 parameters using the ISO Week system, then it would be nonsensical to include a traditional Gregorian year with the other data.

In any event, I don't mind if people feel strongly that it needs qualified despite the class name already implying the IsoWeek format domain. However, if it is further qualified, I strongly object to perpetuating the misuse of the nonsense term "iso year". most of the ISO 8601 specified formats have a year that agrees with the traditional Gregorian year. Only the ISO Week Format year differs.

@ShawnSteele what is your suggestion here? just name it as "year" and not isoYear"?

@tarekgh - For DateTimeOffset, I think that's a fair item for consideration. Folks may be interested in the ISO 2007-04-05T12:30-02:00 type format, which includes a timezone. I'd strongly encourage interchange using UTC, but dates/times aren't always that well behaved :(

@ShawnSteele what is your suggestion here? just name it as "year" and not isoYear"?

@tarekgh Yes, just "year". The "IsoWeek" class provides sufficient context. If people feel that "IsoWeek" is still ambiguous, then I'd suggest "IsoWeekFormat" or some such that makes it more explicit that this is all about the ISO 8601 Week Format.

There is no scenario where it makes sense to pass in a non-iso-week-format year along with an ISO week format week and iso week format day.

For DateTimeOffset, I think that's a fair item for consideration. Folks may be interested in the ISO 2007-04-05T12:30-02:00 type format, which includes a timezone. I'd strongly encourage interchange using UTC, but dates/times aren't always that well behaved :(

I don't think so. we are currently supporting DateTime.ToString("o") which gives the ISO format according to DateTime (and DateTimeOffset). I am not expecting to see Iso week format including the time zone info. does the standard support such format?

ok, based on the last discussion here is the proposal:

C# 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); } }

would that be reasonable for all of you?

Why does the proposed API not include a day of week?

public static int GetDayOfWeek(DateTime time);

If I want to format a date in the ISO Week Format, this class provides the year part of the ISO week format "2018" and the week number "17", but there is no method to return the day of week "7" for this Sunday.

An analogy would be like a Gregorian date information class that provided the year, the month, but not the date (day of month).

I totally get that this is very similar to the DateTime.DayOfWeek - however the ISO day of week is offset from the enum and is a number, not an enum. The code is "straightforward", but would always require munging, something like

int isoWeekDay = DateTime.DayOfWeek == 0 ? 7 : (int)DateTime.DayOfWeek;

which just seems pretty clumsy, especially if the IsoWeek class is already there.

I don't think so. we are currently supporting DateTime.ToString("o") which gives the ISO format according to DateTime (and DateTimeOffset).

@tarekgh - yes, the standard supports that format. https://en.wikipedia.org/wiki/ISO_8601#Combined_date_and_time_representations

"Either basic or extended formats may be used, but both date and time must use the same format. The date expression may be calendar, week, or ordinal, and must use a complete representation."

Why does the proposed API not include a day of week?

Gregorian calendar already has this. I am not really seeing why we need to do that? as I mentioned before, we can look at providing formatting options separately from this proposal. so no need to provide API like what you are proposing. people can still convert Sunday to 7 if they need to do so. Would be overkilling providing such API to do so.

yes, the standard supports that format. https://en.wikipedia.org/wiki/ISO_8601#Combined_date_and_time_representations

I am not interpreting the paragraph you are talking about the same. there is a difference between representing the date and formatting it. I am seeing the ISO date formatting always uses year and month and days (in addition to time and time zone info) but I am not seeing it anywhere you can use a week number in this ISO format. https://www.iso.org/iso-8601-date-and-time-format.html

ISO 8601 tackles this uncertainty by setting out an internationally agreed way to represent dates:

YYYY-MM-DD

You have me confused by "representing the date" vs "formatting it". All of the ISO Week Dates are representable by DateTimes and don't need this class. The ISO Week Formats are primarily a convention for formatting Gregorian dates in a manner that is convenient for some businesses.

The contexts I am used to seeing these dates is in a "weeks for a project" or "day" timescale, however 2018-W17-05T10:00:00+02:00 (and 2018-117T10:00:00+02:00) are certainly permitted by the standard. Sure, it could be represented in the Y-M-D format, however ISO 8601 doesn't required the YMD form for the date portion and allows users of the standard to choose how they want to represent the date part of a complete Date+Time format. (Users may also choose to use UTC or other time representations as well).

Sure, many RFCs and other standards specify the 2018-04-27 format for consistency and portability, however ISO 8601 isn't that constrained.

however ISO 8601 isn't that constrained.

could you please send me the link from the ISO standard allow that (I mean allowing format like 2018-W17-05T10:00:00+02:00)? I appreciate your help here.

Also, I am interested if you can write some code sample how you expect people will write code to use our proposed APIs with DateTimeOffset? I mean, when DateTimeOffset would be more beneficiary than DateTime? or in another word, when our new proposed APIs will care about the time at all?

I don鈥檛 think DateTimeOffset needs addressed right now. We could use DateTime and see if people clamor for a DateTimeOffset. If so, it could be added later. I don鈥檛 think that should block this work.

I'm fine with the proposal you posted last.

Thanks @ShawnSteele

I am going to update the proposal at the top with the following:

C# 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); } }

And I'll remove the alternative design section as what we have here is much better proposal anyway. Thanks all for your valuable feedback.

That's perfect as far as I'm concerned, @tarekgh. Thanks!

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);
    }
}

@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).

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

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

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

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?

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.

Many thanks!

@tarekgh Is the enhancement in 2.1.1?

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

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.

Thanks! I see 3.0 milestone in PRs.

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.

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

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

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?

Sorry @khellang, I as on the phonw.

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.

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.

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.

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:
```c#
int week = ISOWeek.GetWeekOfYear(new DateTime(2016, 1, 2));

I do need to check something like every time I need to know a week:
```c#
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?
c# public static (int year, int week) GetWeekOfYear(DateTime date);

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.

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

Was this page helpful?
0 / 5 - 0 ratings

Related issues

jkotas picture jkotas  路  3Comments

bencz picture bencz  路  3Comments

EgorBo picture EgorBo  路  3Comments

yahorsi picture yahorsi  路  3Comments

matty-hall picture matty-hall  路  3Comments