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.

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

Fix py3.12 warnings for datetime usage, non-raw regex strings #632

Merged
merged 2 commits into from
Sep 13, 2024

Conversation

smemsh
Copy link
Contributor

@smemsh smemsh commented Aug 21, 2024

After upgrading my python3 from python-3.9 to python-3.12, running the test target causes a bunch of new warnings to be emitted. These are regarding datetime.datetime.utcnow() being deprecated. This is because upstream wants to make changes to naive datetimes (those without tzdata).

Here is the blurb from https://docs.python.org/3/whatsnew/3.12.html:

datetime: datetime.datetime’s utcnow() and utcfromtimestamp() are deprecated and will be removed in a future version. Instead, use timezone-aware objects to represent datetimes in UTC: respectively, call now() and fromtimestamp() with the tz parameter set to datetime.UTC.

There is some discussion of upstream's rationale in python/cpython#81669.

For the fix, I chose to use datetime.timezone.utc rather than datetime.UTC because the latter was only added in python-3.11 and I wanted the tests to work under both python-3.9 and python-3.12.

The following edit sequences were run to produce the changes:

First, test_totals.t had imported all of datetime, unlike all the other files which were importing individual names from datetime. It also did a wildcard import from ext/totals.py which caused a shadowing issue so I made the imports more specific. Now taht file imports datetime like the others, so later mass-edits (below) will work on this file too:

sed -i \
-e 's/^.*totals.*\*/from totals import format_seconds, calculate_totals/' \
-e 's/^import.datetime$/from datetime import datetime, timedelta/' \
-e 's/datetime\.datetime\./datetime./g' \
-e 's/datetime\.timedelta/timedelta/' \
test/test_totals.t

Next, we replace the deprecated utcnow() with a tzinfo-aware call, which can be done directly by giving a zone argument to now() but in other uses, a tzinfo-naive object is obtained, used for some things, and then later its .utcnow() method is run. We had to convert those uses in-place by first localizing:

grep -lsrF \
  -e 'datetime.now().utcnow()' \
  -e 'now.utcnow()' \
| xargs sed -i \
  -e 's/datetime.now().utcnow()/datetime.now(timezone.utc)/g' \
  -e '/import.datetime/a\' \
  -e 'from dateutil import tz' \
  -e 's/^\(from datetime import datetime,\)/\1 timezone,/' \
  -e 's/now.utcnow()/now.replace(tzinfo=tz.tzlocal()).astimezone(timezone.utc)/'

Finally, I removed the tzlocal import from files that didn't need it because they didn't do a conversion from naive objects to a aware objects (in the transform above):

for file in $(grep -lsr 'from.dateutil.import.tz$' test)
do grep -q tzlocal $file || sed -i '/^from.dateutil.import.tz$/d' $file
done

Incidentally, there were some regexes in summary.t that are not raw-quoted, generating a warning on embedded \d in non-raw quoted strings. These were probably already warnings before. They were fixed en passant in a separate commit.

Please apply, thanks.

smemsh added a commit to smemsh/timewarrior that referenced this pull request Aug 21, 2024
Python upstream is trying to eliminate tz-naive date functions that
imply anything to do with a timezone, even UTC.  They have deprecated
datetime.datetime.utcnow() in Python 3.12 and thus running tests emits
many warnings for us.

We switch to instantiate datetime objects taht are intended to reflect
UTC to have a timezone, or if we instantiate naive ones and then later
convert, we do the full conversion.

After the changes, the tests still work on Python 3.9, but now also on
Python 3.12 without warnings.

See PR description for GothenburgBitFactory#632 for more details.

Signed-off-by: Scott <[email protected]>
@smemsh smemsh marked this pull request as draft August 21, 2024 02:25
@smemsh smemsh marked this pull request as ready for review August 21, 2024 02:36
@smemsh smemsh changed the title Fix py12 warnings for datetime usage, non-raw regex strings Fix py3.12 warnings for datetime usage, non-raw regex strings Aug 21, 2024
@lauft lauft self-assigned this Aug 23, 2024
smemsh added a commit to smemsh/timewarrior that referenced this pull request Aug 24, 2024
Python upstream is trying to eliminate tz-naive date functions that
imply anything to do with a timezone, even UTC.  They have deprecated
datetime.datetime.utcnow() in Python 3.12 and thus running tests emits
many warnings for us.

We switch to instantiate datetime objects taht are intended to reflect
UTC to have a timezone, or if we instantiate naive ones and then later
convert, we do the full conversion.

After the changes, the tests still work on Python 3.9, but now also on
Python 3.12 without warnings.

See PR description for GothenburgBitFactory#632 for more details.

Signed-off-by: Scott <[email protected]>
Python warns about "\d" and friends that aren't in raw quotes, so we add
the 'r'aw string-qualifier.

Signed-off-by: Scott Mcdermott <[email protected]>
Python upstream is trying to eliminate tz-naive date functions that
imply anything to do with a timezone, even UTC.  They have deprecated
datetime.datetime.utcnow() in Python 3.12 and thus running tests emits
many warnings for us.

We switch to instantiate datetime objects taht are intended to reflect
UTC to have a timezone, or if we instantiate naive ones and then later
convert, we do the full conversion.

After the changes, the tests still work on Python 3.9, but now also on
Python 3.12 without warnings.

See PR description for GothenburgBitFactory#632 for more details.

Signed-off-by: Scott Mcdermott <[email protected]>
@lauft lauft merged commit 05724a9 into GothenburgBitFactory:develop Sep 13, 2024
17 checks passed
@lauft
Copy link
Member

lauft commented Sep 13, 2024

Thanks ❤️

@smemsh smemsh deleted the fix-tests-py12-warns branch September 14, 2024 02:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants