Found a bug in _strptime.py

[cnobile2012]

Bug report

Bug description:

In the code section below, the conditional if julian is not None and julian <= 0: will never be reached, because the condition above it if julian is None and weekday is not None: precludes it. I think it was indented on error and should work find if unindented. This code is on lines 500 to 513 in Python 3.12.

    if julian is None and weekday is not None:
        if week_of_year is not None:
            week_starts_Mon = True if week_of_year_start == 0 else False
            julian = _calc_julian_from_U_or_W(year, week_of_year, weekday,
                                                week_starts_Mon)
        elif iso_year is not None and iso_week is not None:
            datetime_result = datetime_date.fromisocalendar(iso_year, iso_week, weekday + 1)
            year = datetime_result.year
            month = datetime_result.month
            day = datetime_result.day
        if julian is not None and julian <= 0:
            year -= 1
            yday = 366 if calendar.isleap(year) else 365
            julian += yday

CPython versions tested on:

3.12

Operating systems tested on:

Linux

[StanFromIreland]

https://github.com/python/cpython/blob/main/Lib/_strptime.py#L618-L633

It may be calculated in the line:

https://github.com/python/cpython/blob/main/Lib/_strptime.py#L623

[brianschubert]

because the condition above it if julian is None and weekday is not None: precludes it

julian may be re-assigned after the julian is None check if the branch with julian = _calc_julian_from_U_or_W(...) is taken. Does the logic make sense under that perspective? If something is wrong, an example case where the function produces an incorrect output would be helpful.

[StanFromIreland]

The code is fine, there is no bug, the statement could be moved up but that would really just be a cosmetic change IMO.

[cnobile2012]

Yes, I see what you're talking about. My mistake. Thanks for looking into this so quickly.

[cnobile2012]

Although I see why this is not a bug, I do feel that the code segment should be moved up to after julian is set. It not only would feel better, but it wouldn't be executed if the elif was True, thus saving time.

[brianschubert]

@cnobile2012 If this function were being written today, that might be preferable. But for existing code, there's a consensus that these sorts of changes aren't worth it. Any change takes core time to review, can create backport conflicts, and runs the risk of introducing new bugs. There need to be strong benefits to go with to justify the cost. Generally cosmetic changes and micro-performance optimizations don't make the cut.

[cnobile2012]

Hey, no problem.
I found this because I am rewriting the datetime package for the Badí' Calendar, which is an Astronomical Self-correcting Solar calendar, so a lot of things needed to change, mostly dealing with how leap years are determined and when the year starts.
Anyway, thanks for the prompt attention to this NON-bug.