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

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

Open
2 tasks done
kaushalpartani opened this issue Sep 26, 2024 · 3 comments
Open
2 tasks done

Comments

@kaushalpartani
Copy link

  • I am on the latest Pendulum version.
    pendulum==3.0.0
  • I have searched the issues of this repo and believe that this is not a duplicate.
    There are no current open issues. The closest one I could find is one that was opened in 2020. Most of the issues with this property have to do with negative values; however this issue seems to be more about miscounting
  • OS version and name: MacOS Sequoia
  • Pendulum version: 3.0.0

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
Copy link

stripedpumpkin commented Oct 8, 2024

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
Copy link
Author

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
Copy link

stripedpumpkin commented Oct 9, 2024

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)

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

No branches or pull requests

2 participants