Update khal to Follow PEP495 Removing Deprecation Warnings

[pjkaufman]

When running khal ... or ikhal ... an issue would arise where warnings about deprecated functions being used (see #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 and event.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.

[pjkaufman]

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!

[pjkaufman]

Note: I did miss the khal at method and need to check and see what needs to be done for that.

[pjkaufman]

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.

[pjkaufman]

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.

[pjkaufman]

@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

2. 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?

[WhyNotHugo]

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.

[WhyNotHugo]

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?

[pjkaufman]

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?

[pjkaufman]

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:]

[RonnyPfannschmidt]

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

[bancsorin10]

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.

[pjkaufman]

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 <#1093 (comment)>, 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>.

[WhyNotHugo]

@bancsorin10 Agreed. A separate PR pinning a working version of pytz is welcome, at least until we have a definitive version of PEP495 support.

[pjkaufman]

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?

[pjkaufman]

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.

[pjkaufman]

For some reason flake8 does not like this. Any ideas on how to fix it?

[bancsorin10]

is this problem still up? Is there a full log of the error. I will look into it and come back

[pjkaufman]

Here is an idea for how to get transition dates without pytz.

[pjkaufman]

For some reason the TZID is being listed as the timezone name (i.e. EDT, CET, etc.) instead of the passed in name for the ZoneInfo (i.e. "UTC", "America/New_York", etc.).

[pjkaufman]

I am not sure if this is correct for the backport as I did not change it while I imported the module instead of a module inside zoneinfo for the try portion of the try except.

[RonnyPfannschmidt]

Suggested change

	from backports import zoneinfo as ZoneInfo
	from backports.zoneinfo import ZoneInfo

[pjkaufman]

I did what it says to do in the documentation I could find: https://pypi.org/project/backports.zoneinfo/

For some reason

from backports.zoneinfo import ZoneInfo

Yields the following warning: Import "backports.zoneinfo" could not be resolved

[WhyNotHugo]

When flake8 runs on Python3.9, this library won't be installed. It's safe to ignore this specific error.

[bancsorin10]

@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.

[Terrance]

@bancsorin10 The issue is likely with tzlocal rather than pytz directly, see my PR linked above.

[pjkaufman]

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: collective/icalendar#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.

[pjkaufman]

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).

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!

[geier]

I don't think there is any documentation for those _ variables apart from the source code

[geier]

But I now see that there is also a C implementation of the module (which probably gets loaded for you). I don't think we can access those transition times if the C implementation gets used, but I'd need to have a look into this.

[geier]

It looks like if we import ZoneInfo like this

In [1]: from zoneinfo._zoneinfo import ZoneInfo
In [2]: berlin = ZoneInfo('Europe/Berlin')
In [3]: berlin._trans_local

we can force to import the python version.

Also, we would need to first upgrade collectivie/icalendar to use tzinfo to completely switch to tzinfo.

But the current vtimezone implementation definitively is a mess and needs an upgrade.

Perhaps it's more valuable, to have a look at how the ZoneInfo implementation loads the timezone, but I'm not sure what the best way forward here is at the moment.

[pjkaufman]

I plan to make a couple of changes yet on the master branch of my fork, but I would like to ask that someone that uses this app takeover the changes from there since I no longer use this app which means that I don't have a good way to verify that it is working correctly beyond the UTs which may not be extensive enough to be considered proof that the app is working, besides the fact that my changes modify them as well.

I would also recommend updating icalendar to allow for use without pytz as that was a major blocker that has drawn out the updating of this repo.

Beyond that, I plan to see about swapping out the logic for timezone info and finishing the rebase of the branch before stopping making changes to my fork.

I do hope that this change gets implemented as it seems necessary. I also hope that the code that has been added here will aid in the transition from pytz.

Please let me know if there would be a problem with me finishing the rebase and making the timezone transition times change before closing out the PR to allow another dev to hopefully take over.

[pjkaufman]

@WhyNotHugo , @geier , I have made several of the changes requested, but I am not able to build it locally and do not have the expertise to figure out the python errors. However since I plan to relinquish the remaining work and allow someone that is familiar with python and is using khal to complete this PR, I do not see resolving the local build problems due to bad dependency installations as an issue.

Hopefully this will PR can serve as a base for making the change over to no longer use pytz. I hope that the dependencies get updated soon to be compliant with no longer using pytz to help this project keep moving forward. Thank you for letting me try my hand at creating this fix.