sdispater / pendulum

Python datetimes made easy
https://pendulum.eustace.io
MIT License
6.28k stars 386 forks source link

week_of_month returns 6, although months can only have 5 weeks max #847

Open kaushalpartani opened 2 months ago

kaushalpartani commented 2 months ago

Issue

Was playing around with the week_of_month property and found some interesting behavior:

The following:

err_dt = pendulum.datetime(2024, 9, 30, tz='America/Los_Angeles')
err_dt.week_of_month

Results in this output:

6

I'm a bit confused by this behavior and believe it should be a bug because September 2024 should have the following week breakdowns:

Week 1: September 1st, Sunday -> September 8th Sunday Week 2: September 8th, Sunday -> September 15th Sunday Week 3: September 15th Sunday -> September 22nd Sunday Week 4: September 22nd Sunday -> September 29th Sunday Week 5: September 29th Sunday -> September 30th Monday (last date in September)

stripedpumpkin commented 1 month ago

I don't about this very case. But depending on which day you take as your week start you could have 6 distinct weeks.

consider september 2024 and monday as your starting day you get: week 1: 01 => Sunday week 2: 02 (Monday) => 08 (Sunday) week 3: 09 => 15 week 4: 16 => 22 week 5: 23 => 29 week 6: 30 => Monday

maybe this has to do with that.

But in any case the "6-week" months is a real thing in any case.

kaushalpartani commented 1 month ago

I see, I wasn't considering the case where we define a starting day. I was strictly considering whatever day the 1st of the month is as the beginning of the "week counter." I guess it boils down to what the intended/expected behavior of this function is.

stripedpumpkin commented 1 month ago

I cannot speak for the intention but the code relies on isoweekday not on weekday so Monday will be considered as day 1 (of any given week):

    @property
    def week_of_month(self) -> int:
        return math.ceil((self.day + self.first_of("month").isoweekday() - 1) / 7)