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.

Sign up for GitHub

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

Add configurable first day of the week #294

Closed
wants to merge 2 commits into from
Closed

Add configurable first day of the week #294

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6ce5b03
Add configurable first day of the week
sandrobonazzola Feb 2, 2023
d74d90c
tests: add tests for general/week option
sandrobonazzola Feb 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions did/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
from datetime import timedelta

from dateutil.relativedelta import FR as FRIDAY
from dateutil.relativedelta import MO as MONDAY
from dateutil.relativedelta import relativedelta as delta

from did import utils
Expand Down Expand Up @@ -125,6 +124,17 @@ def quarter(self):
f"Invalid quarter start '{month}', should be integer.")
return month

@property
def week(self):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this called week when it is really the first day of the week?

Suggested change
def week(self):
def first_day_of_week(self) -> int:

""" The first day of the week, 0 (Monday) by default"""
week = self.parser.get("general", "week", fallback=0)
try:
week = int(week) % 7
Copy link
Collaborator

@kwk kwk Feb 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest to have a string in the config rather than an int. Transferring to an int internally is fine though. I think of something like this:

#!/bin/env python

WEEKDAYS = ["Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday", "Sunday"]
config_first_day_of_week = "WednES"

# Default first day of the week:
first_day_of_week = 0

# This should allow anything from: "Mon", "mon", "mond", "monda", "monday", "MON",
# but not "mo", "MO" or "M" because it is too short.
if len(config_first_day_of_week) > 3:
    for idx, weekday in enumerate(WEEKDAYS):
        if weekday.lower().startswith(config_first_day_of_week.lower()):
            first_day_of_week = idx

print(first_day_of_week)

except ValueError as exc:
raise ConfigError(
f"Invalid week start '{week}', should be integer.") from exc
return week

@property
def email(self):
""" User email(s) """
Expand Down Expand Up @@ -256,14 +266,18 @@ def __sub__(self, subtrahend):
@staticmethod
def this_week():
""" Return start and end date of the current week. """
since = TODAY + delta(weekday=MONDAY(-1))
since = TODAY
while since.weekday() != Config().week:
since -= delta(days=1)
until = since + delta(weeks=1)
return Date(since), Date(until)

@staticmethod
def last_week():
""" Return start and end date of the last week. """
since = TODAY + delta(weekday=MONDAY(-2))
since = TODAY - delta(weeks=1)
while since.weekday() != Config().week:
since -= delta(days=1)
until = since + delta(weeks=1)
return Date(since), Date(until)

Expand Down
4 changes: 4 additions & 0 deletions docs/config.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,17 @@ Minimum config file should contain at least a ``general`` section
with an email address which will be used for searching. Option
``width`` specifies the maximum width of the report, ``quarter``
can be used to choose a different start month of the quarter.
Option ``week`` can be used to choose a different first day of the week:
defaults to ``0`` which is Monday;
other common setting for this option would be ``6`` which is Sunday.
The ``separator`` and ``separator_width`` options control the
character used, and width of the separator between users::

[general]
email = Petr Šplíchal <psplicha@redhat.com>
width = 79
quarter = 1
week = 0
separator = #
separator_width = 20

Expand Down
20 changes: 17 additions & 3 deletions tests/test_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,20 @@ def test_Date_period():
assert str(since) == "2014-01-01"
assert str(until) == "2015-01-01"
assert period == "the last year"
# This week in Israel
for argument in ["", "week", "this week"]:
Config(config="[general]\nweek = 6")
since, until, period = Date.period(argument)
assert str(since) == "2015-09-27"
assert str(until) == "2015-10-04"
assert period == "the week 39"
# Last week in Israel
for argument in ["last", "last week"]:
Config(config="[general]\nweek = 6")
since, until, period = Date.period(argument)
assert str(since) == "2015-09-20"
assert str(until) == "2015-09-27"
assert period == "the week 38"
# Adding and subtracting days
assert str(Date('2018-11-29') + 1) == '2018-11-30'
assert str(Date('2018-11-29') + 2) == '2018-12-01'
Expand Down Expand Up @@ -267,7 +281,7 @@ def test_get_token_plain_different_name(self):
token = str(uuid4())
config = {"mytoken": token}
self.assertIsNone(get_token(config))
self.assertEqual(get_token(config, token_key="mytoken"), token)
self.assertEqual(get_token(config, token_key="mytoken"), token) # nosec
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this slipped in from your other PR #297 .

Suggested change
self.assertEqual(get_token(config, token_key="mytoken"), token) # nosec
self.assertEqual(get_token(config, token_key="mytoken"), token)


def test_get_token_file(self):
""" Test getting a token from a file """
Expand All @@ -278,7 +292,7 @@ def test_get_token_file(self):

def test_get_token_file_empty(self):
""" Test getting a token from a file with just whitespace. """
token_in_file = " "
token_in_file = " " # nosec
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this slipped in from your other PR #297 .

Suggested change
token_in_file = " " # nosec
token_in_file = " "

with self.get_token_as_file(token_in_file) as filename:
config = {"token_file": filename}
self.assertIsNone(get_token(config))
Expand All @@ -296,7 +310,7 @@ def test_get_token_file_different_name(self):
token_in_file = str(uuid4())
with self.get_token_as_file(token_in_file) as filename:
config = {"mytoken_file": filename}
self.assertEqual(
self.assertEqual( # nosec
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this slipped in from your other PR #297 .

Suggested change
self.assertEqual( # nosec
self.assertEqual(

get_token(
config,
token_file_key="mytoken_file"),
Expand Down