khal
khal copied to clipboard
Update khal to Follow PEP495 Removing Deprecation Warnings
When running khal ...
or ikhal ...
an issue would arise where warnings about deprecated functions being used (see https://github.com/pimutils/khal/issues/1092). This was not a critical change that was needed, but it was more to remove the warnings as an eye sore for myself and those that would not be able to do so on their own.
Changes Made:
- Updated AUTHORS.txt as per the documentation for contributing
- Imported and used ZoneInfo in order to assign a timezone where the warnings were being thrown (2 locations in
khalendar.py
andevent.py
though I decided to add two more locale format updates in one of the methods here)
I had to convert the timezone info to a string just in case it was something else because when I did not do so, I got errors.
Also, I did initially try to change all of the localize
methods to use ZoneInfo, but that started causing test cases to fail for certain files, so I opted to just fix the visible issue.
I am newish to python, but have used it to write programs or scripts before. So please let me know if there is something I missed, a better way to do things, or if this PR should be more extensive.
Here is the source I am using for determine how to convert code to the PEP495: https://pytz-deprecation-shim.readthedocs.io/en/latest/migration.html
Note: there is no longer a built in list of transition dates for daylight savings times, so it will need to be decided how to handle this especial since the standards for this have changed several times.
Correct me if I am wrong, but it looks like pre-commit.ci
has not been added to the actual ci yet since the PR is currently pending a merge. Is there a way I should modify my branch to account for this, or do I need to wait on that other PR which has been waiting for approval for 2 months?
Thank you for letting me know!
Note: I did miss the khal at
method and need to check and see what needs to be done for that.
Note: I did miss the
khal at
method and need to check and see what needs to be done for that.
This was taken care of earlier. I am now just waiting on guidance regarding the pre-commit issue.
I just realized that there are other places where these warnings are displayed as well. I am going to try to fix as many of them as I can, but this may be above my understanding level. The timezone change is simple enough, but the problem seems to arise when dealing with all of these different icalendar attributes and how the new way of doing these things should be done in the current standard.
@WhyNotHugo , PEP495 no longer has transition dates for when going into or out of daylight savings (https://pytz-deprecation-shim.readthedocs.io/en/latest/migration.html#semantic-differences-between-is-dst-and-fold). How would we like to handle that?
Possible Solutions:
- I can try to estimate the DST days for years since 2007 using an algorithm for the standards set then (https://www.timeanddate.com/time/change/usa)
- We could extend this to other date ranges as needed
- we can hard code our own list of when DST happened in each year
Also BST would need a similar solution.
What do you think about this?
Though I would think suppressing dependency print output would probably be bad practice.
Agreed.
Warnings in dependencies need to be addressed in the dependencies themselves. Problem is, some of our dependencies are growing stale or not supporting modern pythons.
On Sat, 23 Oct 2021, at 17:37, Peter Kaufman wrote:
@WhyNotHugo https://github.com/WhyNotHugo , PEP495 no longer has transition dates for when going into or out of daylight savings (https://pytz-deprecation-shim.readthedocs.io/en/latest/migration.html#semantic-differences-between-is-dst-and-fold). How would we like to handle that?
Possible Solutions:
- I can try to estimate the DST days for years since 2007 using an algorithm for the standards set then (https://www.timeanddate.com/time/change/usa)
- We could extend this to other date ranges as needed
- we can hard code our own list of when DST happened in each year Also BST would need a similar solution.
What do you think about this?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pimutils/khal/pull/1093#issuecomment-950169788, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFSNOZECOMZJE5LDQMJRLTUILJENANCNFSM5GQNTTHA. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
Sorry, I'm not fully understanding here. What exactly is it that's missing on PEP495?
On Sat, 23 Oct 2021, at 17:37, Peter Kaufman wrote: @WhyNotHugo https://github.com/WhyNotHugo , PEP495 no longer has transition dates for when going into or out of daylight savings (https://pytz-deprecation-shim.readthedocs.io/en/latest/migration.html#semantic-differences-between-is-dst-and-fold). How would we like to handle that? Possible Solutions: 1. I can try to estimate the DST days for years since 2007 using an algorithm for the standards set then (https://www.timeanddate.com/time/change/usa) * We could extend this to other date ranges as needed 1. we can hard code our own list of when DST happened in each year Also BST would need a similar solution. What do you think about this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1093 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFSNOZECOMZJE5LDQMJRLTUILJENANCNFSM5GQNTTHA. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub. Sorry, I'm not fully understanding here. What exactly is it that's missing on PEP495?
@WhyNotHugo
The issue is located here: https://github.com/pimutils/khal/blob/master/khal/khalendar/event.py#L856-L901
The first issue is that we are using the transition times defined by pytz which no longer exist when the timezone passed in is a ZoneInfo. Here is where we get that information currently: https://github.com/pimutils/khal/blob/master/khal/khalendar/event.py#L866-L868.
Then we use _tzinfos
to help determine our RDATE
and timezone information for our events. I am not sure what the equivalent is in PEP495 so we might not be able to use the current system for determining BST and DST. If we want to fix this, we could have to swap over to a new way of doing things.
The nice thing is that time additions to a datetime will show a change in fold, but will not help us with our checks in this are of code (at least not that I am aware of).
Does that help clarify the issue?
I think I have something for DST times that are newer than 2007 (I think I can add other date ranges as well, it is just that the algorithm for doing so will need to be modified. I am having issues with DST transition times. Is there a way outside of pytz to do determine its transition times?
Here is what I am thinking for the new create_timezone method:
def create_timezone(tz, first_date=None, last_date=None):
...
# TODO last_date = None, recurring to infinity
first_date = dt.datetime.today() if not first_date else to_naive_utc(first_date)
# TODO: determine if there is a way to do this for BST pjk
# check to see if timezone is a part of dst by checking if before or after the transition it has an offset
if first_date.dst() == dt.timedelta(0) and first_date.replace(fold= 1).dst() == dt.timedelta(0):
return _create_timezone_static(tz)
last_date = dt.datetime.today() if not last_date else to_naive_utc(last_date)
timezone = icalendar.Timezone()
timezone.add('TZID', tz)
transition_times = _get_transition_dates_dst_for_daterange(tz, first_date.replace(tzinfo=tz), last_date.replace(tzinfo=tz))
timezones = {}
for index in range(1, len(transition_times)):
name = transition_times[index].tzname()
if name in timezones:
ttime = transition_times[index].replace(tzinfo=None)
if 'RDATE' in timezones[name]:
timezones[name]['RDATE'].dts.append(
icalendar.prop.vDDDTypes(ttime))
else:
timezones[name].add('RDATE', ttime)
continue
if transition_times[index].dst() != dt.timedelta(0):
subcomp = icalendar.TimezoneDaylight()
else:
subcomp = icalendar.TimezoneStandard()
subcomp.add('TZNAME', name)
subcomp.add(
'DTSTART',
tz.fromutc(transition_times[index]).replace(tzinfo=None) # Note: for some reason this is using EDT instead of the zone info name for the value here, I need to figure out why as this is incorrect
)
subcomp.add('TZOFFSETTO', transition_times[index].utcoffset())
subcomp.add('TZOFFSETFROM', transition_times[index - 1].utcoffset())
timezones[name] = subcomp
for subcomp in timezones.values():
timezone.add_component(subcomp)
return timezone
Here is the DST algorithm (this is more a proof of concept than an actual final algorithm):
def _get_transition_dates_dst_for_daterange(tz, start_date, end_date):
#TODO: add date range based algorithms for determining the correct dst time zones where rules exist pjk
if start_date.year < 2007:
msg = (
f"Cannot work with start dates prior to 2007 at this time. Date supplied was" + str(start_date) + "."
)
logger.fatal(msg)
raise FatalError( # because in ikhal you won't see the logger's output
msg
)
year = start_date.year-1
years = []
while year <= end_date.year+1:
years.append(year)
year +=1
index_of_first_transition_before_start_date = 0
index = 0
transition_times = []
for year in years:
marchCal = calendar.monthcalendar(year, 3)
first_week = marchCal[0]
second_week = marchCal[1]
third_week = marchCal[2]
if first_week[calendar.SUNDAY]:
dst_begins = second_week[calendar.SUNDAY]
else:
dst_begins = third_week[calendar.SUNDAY]
dst_start = dt.datetime(year, 3, dst_begins, 2, tzinfo= tz)
transition_times.append(dst_start)
if dst_start < start_date:
index_of_first_transition_before_start_date = index
if end_date < dst_start:
break
novemberCal = calendar.monthcalendar(year, 11)
first_week = novemberCal[0]
second_week = novemberCal[1]
if first_week[calendar.SUNDAY]:
dst_ends = first_week[calendar.SUNDAY]
else:
dst_ends = second_week[calendar.SUNDAY]
dst_end = dt.datetime(year, 11, dst_ends, 2, tzinfo= tz)
transition_times.append(dst_end)
if end_date < dst_end:
break
index +=1
return transition_times[index_of_first_transition_before_start_date:]
as the vtimezone stuff seems daunting i started a conversation on twitter my understanding is that one should get the vtimezone from the underlying data
i may be able to create a POC of that by the weekend
Greetings, as of my understanding the warnings are from pytz
newest version. Is it worth it to make a smaller PR to fix the pytz version in the setup.py to a version that is working so the build is not broken(I think this warning introduced other unexpected behaviours - e.g. fresh clone and build - trying to create events - cannot change the hours range). For me it is not clear the state of this PR if there is anything I can help out with lmk I wish to contribute and accelerate this PR.
That may work, but the change happened as of python 3.9, so I think we would have to discontinue the use of python 3.9 in order for that to work, but I am aomewhat new to python. I still have some changes I have not committed because they are not ready yet, but I can see about adding to it this weekend
On Fri, Oct 29, 2021, 6:46 AM Turt7le @.***> wrote:
Greetings, as of my understanding the warnings are from pytz newest version. Is it worth it to make a smaller PR to fix the pytz version in the setup.py to a version that is working so the build is not broken(I think this warning introduced other unexpected behaviours - e.g. fresh clone and build - trying to create events - cannot change the hours range). For me it is not clear the state of this PR if there is anything I can help out with lmk I wish to contribute and accelerate this PR.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/pimutils/khal/pull/1093#issuecomment-954643459, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHEDPO4MPST4CMISDL7NHSLUJKCSTANCNFSM5GQNTTHA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
@bancsorin10 Agreed. A separate PR pinning a working version of pytz is welcome, at least until we have a definitive version of PEP495 support.
I made some changes to remove a lot of the pytz method calls and it seems to work for some things. The unit tests are failing (41 of them to be exact) and some of that is because I have not implemented logic for DST dates prior to 2007, I have no logic in place for BST dates, and the TZID is being listed at the timezone name instead of the ZoneInfo key.
However, I tried to commit my changes, but I got several errors from the precommit checks. They currently are preventing me from actually commiting to my own repo, so I will just list out some of the errors and ask for ideas on how to fix them.
The first issue it is really complaining about is the following:
try:
from zoneinfo import ZoneInfo
except ImportError:
from backports import zoneinfo as ZoneInfo
The precheck says the following about it:
khal/utils.py:29: error: Incompatible import of "ZoneInfo" (imported name has type Module, local name has type "Type[ZoneInfo]")
tests/utils.py:7: error: Incompatible import of "ZoneInfo" (imported name has type Module, local name has type "Type[ZoneInfo]")
Any ideas on how to fix this?
I figured out how to get past the the precommit check failures for flake8
, but I am not sure what I need to do to fix the actual errors. Please let me know what I should do to fix them. I am also unaware of what we should do to correctly replace transition time checking and BST time checking. Any ideas are welcome.
@bancsorin10 Agreed. A separate PR pinning a working version of pytz is welcome, at least until we have a definitive version of PEP495 support.
from my testing I could not pinpoint the problem. I used python 3.9 with pytz version as old as 2013, and python 3.7.7 with the newest versions of pytz. It might be a combination of both or my system may just be too corrupted even tho I made the builds with a virtual python environment. I will try a few more versions and if I find something before this pr goes live I'll let you know.
@bancsorin10 The issue is likely with tzlocal rather than pytz directly, see my PR linked above.
I have been able to determine that the TZID issue is probably an icalendar issue where it is not handling ZoneInfo and date util timezone names correctly: https://github.com/collective/icalendar/issues/291
I have suggested a change and can make a PR and see what happens, but I am not sure what to do if the issue is in icalendar and they do not choose to fix it.
I can definitely continue working on these changes, but I will probably need some help with fixing the unit tests as I am not the most familiar with timezone information especially when stored using ical.
Close-opened so CI will run, since GitHub seems to have dropped the last logs.
Having a look at this now.
It's a bit unfortunate, that the only way to localize an existing datetime seems to be to use .replace(tzinfo=newtz)
, as this doesn't except like pytz's localize() did, if the existing datetime is already timezone aware.
We might want to remove pytz from icalendar first, so we can get rid of all the ugly ZoneInfo(str(timezone))
s.
Thank you, @pjkaufman! This really helps!
I rebased this PR and fixed the first outstanding issue to #1125 for now.
Especially the timezone create_timezone()
is a really ugly piece of code I never had the chance to properly clean up. I'll try to see if I can find the time to fix this outstanding issues (no promises).
Regarding creating the VTIMEZONEs:
we can probably create something similar as before from ZoneInfo._ttinfos
, Zoneinfo._trans_utc
and Zoneinfo._fixed_offsets
. But we could use the chance to read the timezone information values and extract the rrules from those (would lead to smaller VTIMEZONEs).
Regarding creating the VTIMEZONEs:
we can probably create something similar as before from
ZoneInfo._ttinfos
,Zoneinfo._trans_utc
andZoneinfo._fixed_offsets
. But we could use the chance to read the timezone information values and extract the rrules from those (would lead to smaller VTIMEZONEs).
That is quite interesting. I was unaware that this was a thing. I can see about adding it if you would like.
We might want to remove pytz from icalendar first, so we can get rid of all the ugly
ZoneInfo(str(timezone))
s.
This would be great if it could be updated since it looks like pytz is being removed/deprecated.
Regarding creating the VTIMEZONEs: we can probably create something similar as before from
ZoneInfo._ttinfos
,Zoneinfo._trans_utc
andZoneinfo._fixed_offsets
. But we could use the chance to read the timezone information values and extract the rrules from those (would lead to smaller VTIMEZONEs).That is quite interesting. I was unaware that this was a thing. I can see about adding it if you would like.
Yes, please go ahead.
@pjkaufman you can also pull the rebased branch into this.
Also the ics project seems to be able to create VTIMEZONEs, perhaps it's valuable to have a look at their approach: https://github.com/ics-py/ics-py/blob/main/src/ics/timezone/init.py
Regarding creating the VTIMEZONEs: we can probably create something similar as before from
ZoneInfo._ttinfos
,Zoneinfo._trans_utc
andZoneinfo._fixed_offsets
. But we could use the chance to read the timezone information values and extract the rrules from those (would lead to smaller VTIMEZONEs).That is quite interesting. I was unaware that this was a thing. I can see about adding it if you would like.
Yes, please go ahead.
Could you please link out to the documentation for ZoneInfo._ttinfos
, Zoneinfo._trans_utc
and Zoneinfo._fixed_offsets
as I cannot seem to find it? Without the documentation and the fact that python 3.9.7 keeps saying that
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/usr/lib/python3.9/zoneinfo/__init__.py", line 27, in __getattr__
raise AttributeError(f"module {__name__!r} has no attribute {name!r}")
AttributeError: module 'zoneinfo' has no attribute '_ttinfos'
It may be that I am just misunderstanding what you mean here size it it unclear whether it is the zone info instance or the module that has the data on it. Thanks for the help!