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

Update type annotations to use | operator #3323

Merged

Conversation

davidculley
Copy link
Contributor

Updated Optional and Union type annotations to the | operator.

@davidculley
Copy link
Contributor Author

This pull request fails because the | operator requires Python 3.10 or newer, but the Python version installed (using Pyenv) is 3.9.18.

I think we need to update these two files:

The Python releases can be found here.

  1. Do we want to update Python from 3.9 to something newer?
  2. If yes, can anyone do this for me please?

@davidculley davidculley force-pushed the feature/update-type-annotations-optional branch from 497f342 to 3a02b4b Compare July 25, 2024 15:34
@dae
Copy link
Member

dae commented Jul 25, 2024

We cannot upgrade Python at this time, due to #3081. | annotations are supported on Python 3.9, provided the following line is found at the top of the file:

from __future__ import annotations

@dae
Copy link
Member

dae commented Jul 25, 2024

While you are welcome to use CI for testing if you wish, you may find ./check on your local machine gives you a faster feedback cycle. I find it useful to put it into a git hook, so it happens automatically on push, and aborts the push if the check fails.

@davidculley davidculley force-pushed the feature/update-type-annotations-optional branch from 2139ef3 to 4bd936b Compare July 25, 2024 16:14
@davidculley davidculley marked this pull request as ready for review July 25, 2024 16:17
@davidculley
Copy link
Contributor Author

I'm completely unfamiliar with the toolchain Anki uses and with Rust, but I'm slowly figuring things out. 🙂 Got the PR to pass! Thank you for your help!

Such a git hook sounds very useful. (I already use pre-commit.) Do you mind sharing the script?

@dae
Copy link
Member

dae commented Jul 25, 2024

I have this at the top of pre-push:

set -e
if [ "$SKIP_CI" != "1" ]; then
    ./check
fi
exit

It's not perfect - it runs when I do things like try to push a tag deletion as well. But works well for the common case.

@dae dae merged commit 0743e6e into ankitects:main Jul 26, 2024
1 check passed
@davidculley davidculley deleted the feature/update-type-annotations-optional branch July 26, 2024 21:26
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