Added new feature for showing time-zone difference in hrs #28
Conversation
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
| @@ -29,13 +31,14 @@ def __init__( | |||
| ).astimezone() | |||
|
|
|||
| self.midnights = [local_midnight] | |||
|
|
|||
| self.diff_dict: Dict[str, str] = {} | |||
There was a problem hiding this comment.
As this is only used inside _get_headers, this assignment should not exist.
| def _get_headers(self) -> List[str]: | ||
| headers: List[str] = [] | ||
| if self.difference: | ||
| self.diff_dict = self.get_difference() |
There was a problem hiding this comment.
As we are not assigning self.diff_dict as an instance variable, and as we will return a list of differences, this should be just:
differences = self.get_differences()| def get_difference(self) -> Dict[str, str]: | ||
| fmt = '%Y-%m-%d %H:%M' | ||
| hour = datetime.now().hour | ||
| diff_dict: Dict[str, str] = {} | ||
| tz0 = datetime.fromisoformat( | ||
| (self.foreign_zones[0] + timedelta(hours=hour)).strftime(fmt), | ||
| ) | ||
| for idx, midnight in enumerate(self.foreign_zones): | ||
| if idx > 0: | ||
| tz1 = datetime.fromisoformat( | ||
| (midnight + timedelta(hours=hour)).strftime(fmt), | ||
| ) | ||
| if tz0 < tz1: | ||
| diff_dict[str(midnight.tzinfo).upper()] = ( | ||
| ' (-' + str(tz1 - tz0).split(':')[0] + ')' | ||
| ) | ||
| else: | ||
| diff_dict[str(midnight.tzinfo).upper()] = ( | ||
| ' (+' + str(tz0 - tz1).split(':')[0] + ')' | ||
| ) | ||
| else: | ||
| tz0 = datetime.fromisoformat( | ||
| (midnight + timedelta(hours=hour)).strftime(fmt), | ||
| ) | ||
| return diff_dict |
There was a problem hiding this comment.
I think that we need some changes here:
- This needs to return just the difference in hours instead of the final string. As we will calculate all differences here, we should return a
List[int], so the method should be calledget_differences.
As fromisoformat was introduced in Python 3.7, we cannot use it, because I want to maintain backwards compatibility, so we need to take another approach and just build datetime objects like this:
tz0 = datetime(
self.midnights[0].year,
self.midnights[0].month,
self.midnights[0].day,
self.midnights[0].hour
)
tz1 = datetime(
midnight.year,
midnight.month,
midnight.day,
midnight.hour,
)Then we can calculate the difference in hours like this:
difference = abs(int(((tz0 - tz1).seconds / 60) / 60))| if args.difference: | ||
| returncode = ComparisonView( | ||
| args.timezone, | ||
| args.zone, | ||
| args.difference, | ||
| args.hour, | ||
| ).print_table() | ||
| else: | ||
| returncode = ComparisonView( | ||
| args.timezone, | ||
| args.zone, | ||
| False, | ||
| args.hour, | ||
| ).print_table() |
There was a problem hiding this comment.
This logic is not necessary, as we can just always pass args.difference to ComparisonView
| '-d', | ||
| '--difference', | ||
| action='store_true', | ||
| help='show difference in hrs', |
There was a problem hiding this comment.
Small change here, hrs should be hours.
| if self.zone: | ||
| headers.append(f'{header} ({midnight.tzname()})') | ||
|
|
||
| if self.difference: | ||
| if idx > 0: | ||
| headers.append( | ||
| f'{header} ({midnight.tzname()})' | ||
| + self.diff_dict[str(midnight.tzinfo).upper()], | ||
| ) | ||
| else: | ||
| headers.append(f'{header} ({midnight.tzname()})') | ||
| else: | ||
| headers.append(f'{header} ({midnight.tzname()})') | ||
| else: | ||
| headers.append(header) |
There was a problem hiding this comment.
This logic should be fixed, as we want to show the difference if --difference or -d is present, it should not depend in any other argument.
There was a problem hiding this comment.
But if the zone is only not there, what difference will it compute?
There was a problem hiding this comment.
The only thing that zone does is show the zone name in the column header, as the argument help says:
There was a problem hiding this comment.
Okay, got it, I will make the changes and get back. I shall ask questions, if needed.
There was a problem hiding this comment.
Okay, great. Of course, If you have any questions please ask! :)
|
Closing it because of inactivity and unresolved issues |
Added new feature for showing time-zone differences with the local and the timezone entered by the user in hours. It also shows whether the user's time is ahead or behind the timezone entered.