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

start_of('week') and day_of_week inconsistent #683

Closed
blksith0 opened this issue Jan 22, 2023 · 2 comments · Fixed by #731
Closed

start_of('week') and day_of_week inconsistent #683

blksith0 opened this issue Jan 22, 2023 · 2 comments · Fixed by #731

Comments

@blksith0
Copy link

If today is Sunday, then start_of('week') will actually go back to the previous Monday.
Yet day_of_week will say 0.

"ISO8601 week starts on Monday"

If this is enforced then it should be consistent.
Thanks

@ndkv
Copy link

ndkv commented Jul 2, 2023

I'm also encountering this issue.

Setting pendulum.week_starts_at(pendulum.SUNDAY) and calling start_of('week') returns today's Sunday.

day_of_week() still returns 0 which is now correct but it doesn't seem to be linked to the pendulum.week_starts_at() setting.

@ndkv
Copy link

ndkv commented Jul 2, 2023

I had a look at day_of_week()'s definition:

    def day_of_week(self):
        """
        Returns the day of the week (0-6).

        :rtype: int
        """
        return self.isoweekday() % 7

and saw that it calls Python's built-in datetime.date.isoweekday() function which, on Python 3.10.11, returns Monday == 1 ... Sunday == 7:

    def isoweekday(self):
        "Return day of the week, where Monday == 1 ... Sunday == 7."
        # 1-Jan-0001 is a Monday
        return self.toordinal() % 7 or 7

The issue is caused by the return self.isoweekday() % 7 in day_of_week(), as it turns isoweekday's output for Sunday (7) into 0.


I see two solutions

  1. replace return self.isoweekday() % 7 with return self.isoweekday() - 1
  2. call datetime.date.weekday() instead as it returns Monday == 0 ... Sunday == 6

Which is preferable?

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

Successfully merging a pull request may close this issue.

2 participants